diff mbox series

rs6000: Use NO_EXPR to cast to MMA pointer types

Message ID 58acc8c2-b6a4-e9ec-c89b-ad841cf759ed@linux.ibm.com
State New
Headers show
Series rs6000: Use NO_EXPR to cast to MMA pointer types | expand

Commit Message

Peter Bergner Sept. 2, 2022, 4:22 p.m. UTC 
When we cast pointers to our opaque MMA pointers, use NOP_EXPR rather
than VIEW_CONVERT_EXPR.

This passed bootstrap and regtesting on powerpc64le-linux with no regressions.
Ok for trunk?

I think this is just a cleanup and not a correctness thing, so I'm assuming a
backport isn't needed?  Or maybe we do to make other potential backports easier?
I'm fine either way.

Peter


gcc/
	* config/rs6000/rs6000-builtin.cc (rs6000_gimple_fold_mma_builtin): Use
	NOP_EXPR for MMA pointer casting.

Comments

Segher Boessenkool Sept. 2, 2022, 4:31 p.m. UTC | #1 
Hi!

On Fri, Sep 02, 2022 at 11:22:07AM -0500, Peter Bergner wrote:
> When we cast pointers to our opaque MMA pointers, use NOP_EXPR rather
> than VIEW_CONVERT_EXPR.

> I think this is just a cleanup and not a correctness thing, so I'm assuming a
> backport isn't needed?  Or maybe we do to make other potential backports easier?
> I'm fine either way.

I wouldn't worry about backports.  If it does make other backports
easier in the future, we can decide to backport this *then*.

Okay for trunk.  Thanks!

(Did you also look at non-MMA VIEW_CONVERT_EXPR uses btw?)


Segher
Peter Bergner Sept. 2, 2022, 5:02 p.m. UTC | #2 
On 9/2/22 11:31 AM, Segher Boessenkool wrote:
> I wouldn't worry about backports.  If it does make other backports
> easier in the future, we can decide to backport this *then*.

Ok.



> (Did you also look at non-MMA VIEW_CONVERT_EXPR uses btw?)

I did.  It seemed they were all related to pointers to vectors and I remember
you mentioning that as one of the reasons for using VIEW_CONVERT_EXPR over
NOP_EXPR, so I left them alone to be safe.



> Okay for trunk.  Thanks!

Ok, pushed to trunk.  Thanks!


Peter
Segher Boessenkool Sept. 2, 2022, 5:23 p.m. UTC | #3 
On Fri, Sep 02, 2022 at 12:02:54PM -0500, Peter Bergner wrote:
> On 9/2/22 11:31 AM, Segher Boessenkool wrote:
> > (Did you also look at non-MMA VIEW_CONVERT_EXPR uses btw?)
> 
> I did.  It seemed they were all related to pointers to vectors and I remember
> you mentioning that as one of the reasons for using VIEW_CONVERT_EXPR over
> NOP_EXPR, so I left them alone to be safe.

Huh?  I have no idea what you mean here.

Casting from one pointer type to another never needs it.  Casting from a
scalar integer type to a pointer type not either AFAIKi.  But I am not a
Gimple expert, all this might be wrong, it isn't documented anywbere :-(


Segher
Peter Bergner Sept. 2, 2022, 6:17 p.m. UTC | #4 
On 9/2/22 12:23 PM, Segher Boessenkool wrote:
> On Fri, Sep 02, 2022 at 12:02:54PM -0500, Peter Bergner wrote:
>> On 9/2/22 11:31 AM, Segher Boessenkool wrote:
>>> (Did you also look at non-MMA VIEW_CONVERT_EXPR uses btw?)
>>
>> I did.  It seemed they were all related to pointers to vectors and I remember
>> you mentioning that as one of the reasons for using VIEW_CONVERT_EXPR over
>> NOP_EXPR, so I left them alone to be safe.
> 
> Huh?  I have no idea what you mean here.
> 
> Casting from one pointer type to another never needs it.  Casting from a
> scalar integer type to a pointer type not either AFAIKi.  But I am not a
> Gimple expert, all this might be wrong, it isn't documented anywbere :-(

Ah, then I misunderstood you and didn't pick up on the non-pointer thing.
...which just goes to show I'm not an expert and someone else should look
at those uses. :-) 

Peter
Richard Biener Sept. 5, 2022, 9:25 a.m. UTC | #5 
On Fri, Sep 2, 2022 at 7:24 PM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> On Fri, Sep 02, 2022 at 12:02:54PM -0500, Peter Bergner wrote:
> > On 9/2/22 11:31 AM, Segher Boessenkool wrote:
> > > (Did you also look at non-MMA VIEW_CONVERT_EXPR uses btw?)
> >
> > I did.  It seemed they were all related to pointers to vectors and I remember
> > you mentioning that as one of the reasons for using VIEW_CONVERT_EXPR over
> > NOP_EXPR, so I left them alone to be safe.
>
> Huh?  I have no idea what you mean here.
>
> Casting from one pointer type to another never needs it.  Casting from a
> scalar integer type to a pointer type not either AFAIKi.  But I am not a
> Gimple expert, all this might be wrong, it isn't documented anywbere :-(

NOP_EXPR is for conversions between types with the same kind
(and pointer-to-integer and integer-to-pointer
conversions when pointer and integer are of the same size).
When used on vectors it converts the vector elements.  When you want to
re-interpret V4SI as V4SF you need VIEW_CONVERT_EXPR (bit_cast),
likewise V4SI interpreted as V16QI needs that.

Think of VIEW_CONVERT as bit_cast and NOP_EXPR as conversion.
Of course for some conversions (like unsigned int to int) you can also
use a VIEW_CONVERT since it's semantically the same.  In those cases
we canonicalize to NOP_EXPR via folding.

Richard.

>
>
> Segher
Segher Boessenkool Sept. 5, 2022, 2:51 p.m. UTC | #6 
On Mon, Sep 05, 2022 at 11:25:21AM +0200, Richard Biener wrote:
> On Fri, Sep 2, 2022 at 7:24 PM Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
> > On Fri, Sep 02, 2022 at 12:02:54PM -0500, Peter Bergner wrote:
> > > On 9/2/22 11:31 AM, Segher Boessenkool wrote:
> > > > (Did you also look at non-MMA VIEW_CONVERT_EXPR uses btw?)
> > >
> > > I did.  It seemed they were all related to pointers to vectors and I remember
> > > you mentioning that as one of the reasons for using VIEW_CONVERT_EXPR over
> > > NOP_EXPR, so I left them alone to be safe.
> >
> > Huh?  I have no idea what you mean here.
> >
> > Casting from one pointer type to another never needs it.  Casting from a
> > scalar integer type to a pointer type not either AFAIKi.  But I am not a
> > Gimple expert, all this might be wrong, it isn't documented anywbere :-(
> 
> NOP_EXPR is for conversions between types with the same kind
> (and pointer-to-integer and integer-to-pointer
> conversions when pointer and integer are of the same size).
> When used on vectors it converts the vector elements.  When you want to
> re-interpret V4SI as V4SF you need VIEW_CONVERT_EXPR (bit_cast),
> likewise V4SI interpreted as V16QI needs that.
> 
> Think of VIEW_CONVERT as bit_cast and NOP_EXPR as conversion.
> Of course for some conversions (like unsigned int to int) you can also
> use a VIEW_CONVERT since it's semantically the same.  In those cases
> we canonicalize to NOP_EXPR via folding.

Thanks for the explanation!

About that last point...  You say VIEW_CONVERT_EXPR is folded to
NOP_EXPR where possible.  Does that happen in all cases / can we depend
on that?  So that in target code like what started this thread the only
real difference is documentation of intent?  (Which never is unimportant
of course!)


Segher
Richard Biener Sept. 5, 2022, 6:37 p.m. UTC | #7 
> Am 05.09.2022 um 16:53 schrieb Segher Boessenkool <segher@kernel.crashing.org>:
> 
> On Mon, Sep 05, 2022 at 11:25:21AM +0200, Richard Biener wrote:
>>> On Fri, Sep 2, 2022 at 7:24 PM Segher Boessenkool
>>> <segher@kernel.crashing.org> wrote:
>>> On Fri, Sep 02, 2022 at 12:02:54PM -0500, Peter Bergner wrote:
>>>> On 9/2/22 11:31 AM, Segher Boessenkool wrote:
>>>>> (Did you also look at non-MMA VIEW_CONVERT_EXPR uses btw?)
>>>> 
>>>> I did.  It seemed they were all related to pointers to vectors and I remember
>>>> you mentioning that as one of the reasons for using VIEW_CONVERT_EXPR over
>>>> NOP_EXPR, so I left them alone to be safe.
>>> 
>>> Huh?  I have no idea what you mean here.
>>> 
>>> Casting from one pointer type to another never needs it.  Casting from a
>>> scalar integer type to a pointer type not either AFAIKi.  But I am not a
>>> Gimple expert, all this might be wrong, it isn't documented anywbere :-(
>> 
>> NOP_EXPR is for conversions between types with the same kind
>> (and pointer-to-integer and integer-to-pointer
>> conversions when pointer and integer are of the same size).
>> When used on vectors it converts the vector elements.  When you want to
>> re-interpret V4SI as V4SF you need VIEW_CONVERT_EXPR (bit_cast),
>> likewise V4SI interpreted as V16QI needs that.
>> 
>> Think of VIEW_CONVERT as bit_cast and NOP_EXPR as conversion.
>> Of course for some conversions (like unsigned int to int) you can also
>> use a VIEW_CONVERT since it's semantically the same.  In those cases
>> we canonicalize to NOP_EXPR via folding.
> 
> Thanks for the explanation!
> 
> About that last point...  You say VIEW_CONVERT_EXPR is folded to
> NOP_EXPR where possible.  Does that happen in all cases / can we depend
> on that?  So that in target code like what started this thread the only
> real difference is documentation of intent?  (Which never is unimportant
> of course!)

You should be able to depend on that, yes.

Richard 

> 
> 
> Segher
diff mbox series

Patch

diff --git a/gcc/config/rs6000/rs6000-builtin.cc b/gcc/config/rs6000/rs6000-builtin.cc
index e6948b9abb7..0d8be996f4e 100644
--- a/gcc/config/rs6000/rs6000-builtin.cc
+++ b/gcc/config/rs6000/rs6000-builtin.cc
@@ -1101,7 +1101,7 @@  rs6000_gimple_fold_mma_builtin (gimple_stmt_iterator *gsi,
 	  || (fncode == RS6000_BIF_DISASSEMBLE_PAIR_V
 	      && TREE_TYPE (TREE_TYPE (dst_ptr)) == vector_pair_type_node))
 	{
-	  tree dst = build_simple_mem_ref (build1 (VIEW_CONVERT_EXPR,
+	  tree dst = build_simple_mem_ref (build1 (NOP_EXPR,
 						   src_type, dst_ptr));
 	  gimplify_assign (dst, src, &new_seq);
 	  pop_gimplify_context (NULL);
@@ -1125,7 +1125,7 @@  rs6000_gimple_fold_mma_builtin (gimple_stmt_iterator *gsi,
 	= rs6000_builtin_decls[rs6000_builtin_info[fncode].assoc_bif];
       tree dst_type = build_pointer_type_for_mode (unsigned_V16QI_type_node,
 						   ptr_mode, true);
-      tree dst_base = build1 (VIEW_CONVERT_EXPR, dst_type, dst_ptr);
+      tree dst_base = build1 (NOP_EXPR, dst_type, dst_ptr);
       for (unsigned i = 0; i < nvec; i++)
 	{
 	  unsigned index = WORDS_BIG_ENDIAN ? i : nvec - 1 - i;
@@ -1151,7 +1151,7 @@  rs6000_gimple_fold_mma_builtin (gimple_stmt_iterator *gsi,
       tree ptr = gimple_call_arg (stmt, 1);
       tree lhs = gimple_call_lhs (stmt);
       if (TREE_TYPE (TREE_TYPE (ptr)) != vector_pair_type_node)
-	ptr = build1 (VIEW_CONVERT_EXPR,
+	ptr = build1 (NOP_EXPR,
 		      build_pointer_type (vector_pair_type_node), ptr);
       tree mem = build_simple_mem_ref (build2 (POINTER_PLUS_EXPR,
 					       TREE_TYPE (ptr), ptr, offset));
@@ -1168,7 +1168,7 @@  rs6000_gimple_fold_mma_builtin (gimple_stmt_iterator *gsi,
       tree offset = gimple_call_arg (stmt, 1);
       tree ptr = gimple_call_arg (stmt, 2);
       if (TREE_TYPE (TREE_TYPE (ptr)) != vector_pair_type_node)
-	ptr = build1 (VIEW_CONVERT_EXPR,
+	ptr = build1 (NOP_EXPR,
 		      build_pointer_type (vector_pair_type_node), ptr);
       tree mem = build_simple_mem_ref (build2 (POINTER_PLUS_EXPR,
 					       TREE_TYPE (ptr), ptr, offset));