Re: Add VIEW_CONVERT_EXPR to operand_equal_p

On Wed, Nov 4, 2015 at 5:50 PM, Jan Hubicka wrote: >> > Are these supposed to be fixed by Richard's change to not use >> > useless_type_conversion for VCE, or is it another issue? >> >> Richard's change not to use useless_type_conversion for VCE was causing >> additional GIMPLE

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

> > Are these supposed to be fixed by Richard's change to not use > > useless_type_conversion for VCE, or is it another issue? > > Richard's change not to use useless_type_conversion for VCE was causing > additional GIMPLE verification failures so I didn't pursue; I can try again, > but all the

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

> Ah - tree_ssa_useless_type_conversion and callers, during gimplification. > I'd like to get rid of it but maybe simply delete the VIEW_CONVERT_EXPR > case from it for now (and return true unconditionally for NON_LVALUE_EXPR). > > Index: gcc/tree-ssa.c >

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

> Are these supposed to be fixed by Richard's change to not use > useless_type_conversion for VCE, or is it another issue? Richard's change not to use useless_type_conversion for VCE was causing additional GIMPLE verification failures so I didn't pursue; I can try again, but all the known

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

> This fails on ia64. gnat.dg/discr44.adb and gnat.dg/discr45.adb are supposed to fail everywhere (and there are also a few ACATS failures everywhere). Sorry for the awkward situation but they are testcases exposing the recent type system breakage. -- Eric Botcazou

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

On Tue, Nov 3, 2015 at 11:25 AM, Eric Botcazou wrote: >> I suggest to re-instantiate the canonical type checks for the aggregate type >> case. > > OK, thanks, this fixes all the known ICEs so far. > > Tested on x86_64-suse-linux, OK for the mainline? Please instead do the

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

> I suggest to re-instantiate the canonical type checks for the aggregate type > case. OK, thanks, this fixes all the known ICEs so far. Tested on x86_64-suse-linux, OK for the mainline? 2015-11-03 Eric Botcazou * gimple-expr.c (useless_type_conversion_p):

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

> > This fails on ia64. > > gnat.dg/discr44.adb and gnat.dg/discr45.adb are supposed to fail everywhere > (and there are also a few ACATS failures everywhere). Sorry for the awkward > situation but they are testcases exposing the recent type system breakage. Are these supposed to be fixed by

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

Eric Botcazou writes: > * gnat.dg/discr45.adb: New test. This fails on ia64. raised CONSTRAINT_ERROR : discr45.adb:19 range check failed #0 <__gnat_rcheck_CE_Range_Check> (file=(system.address) 0x40022e98, line=19) at a-except.adb:1286 #1

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

On Fri, Oct 30, 2015 at 4:19 PM, Jan Hubicka wrote: >> >> Yeah, I suppose we'd need to either build a new function type for each >> variadic call >> then or somehow represent 'fntype' differently (note that function >> attributes also >> need to be preserved). > > Why we can't

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

> Lets go with this patch and hopefully stabilize the tree. I don't think the > vector conversions represent an important case. Unfortunately the patch introduces GIMPLE checking failures in Ada so it will need to be completed/improved. But let's postpone it because we have another class of

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

On October 31, 2015 6:17:35 PM GMT+01:00, Eric Botcazou wrote: >> Lets go with this patch and hopefully stabilize the tree. I don't >think the >> vector conversions represent an important case. > >Unfortunately the patch introduces GIMPLE checking failures in Ada so >it

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

On Thu, Oct 29, 2015 at 4:52 PM, Jan Hubicka wrote: >> On Thu, Oct 29, 2015 at 4:02 PM, Jan Hubicka wrote: >> >> >> >> IMHO it was always wrong/fragile for backends to look at the actual >> >> arguments to >> >> decide on the calling convention. The backends

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

> > But yes, the VIEW_CONVERT "stripping" is a bit fragile and I don't > > remember what exactly we gain from it (when not done on registers). > > I guess gain is really limited to Ada - there are very few cases we do VCE > otherwise. (I think we could do more of them). We can make >

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

> > Yeah, I suppose we'd need to either build a new function type for each > variadic call > then or somehow represent 'fntype' differently (note that function > attributes also > need to be preserved). Why we can't keep fntype as it is, but simply add a new set of parameters to call stmt that

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

> > > But yes, the VIEW_CONVERT "stripping" is a bit fragile and I don't > > > remember what exactly we gain from it (when not done on registers). > > > > I guess gain is really limited to Ada - there are very few cases we do VCE > > otherwise. (I think we could do more of them). We can make > >

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

On Thu, Oct 29, 2015 at 12:31 PM, Richard Biener wrote: > On Thu, Oct 29, 2015 at 12:22 PM, Richard Biener > wrote: >> On Thu, Oct 29, 2015 at 4:39 AM, Jan Hubicka wrote: > Added and comitted now. Thanks.

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

On Thu, Oct 29, 2015 at 12:22 PM, Richard Biener wrote: > On Thu, Oct 29, 2015 at 4:39 AM, Jan Hubicka wrote: >>> > Added and comitted now. >>> >>> Thanks. Now on to the wrong code issues. :-) >>> >>> Up to the change, the useless_type_conversion_p

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

> > IMHO it was always wrong/fragile for backends to look at the actual arguments > to > decide on the calling convention. The backends should _solely_ rely on > gimple_call_fntype and its TYPE_ARG_TYPES here. > > Of course then there are varargs ... (not sure if we hit this here). Yep, you

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

On Thu, Oct 29, 2015 at 4:02 PM, Jan Hubicka wrote: >> >> IMHO it was always wrong/fragile for backends to look at the actual >> arguments to >> decide on the calling convention. The backends should _solely_ rely on >> gimple_call_fntype and its TYPE_ARG_TYPES here. >> >> Of

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

> On Thu, Oct 29, 2015 at 4:02 PM, Jan Hubicka wrote: > >> > >> IMHO it was always wrong/fragile for backends to look at the actual > >> arguments to > >> decide on the calling convention. The backends should _solely_ rely on > >> gimple_call_fntype and its TYPE_ARG_TYPES here.

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

On Thu, Oct 29, 2015 at 4:39 AM, Jan Hubicka wrote: >> > Added and comitted now. >> >> Thanks. Now on to the wrong code issues. :-) >> >> Up to the change, the useless_type_conversion_p predicate was relying on >> structural equivalence via the TYPE_CANONICAL check, now it only

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

> > Added and comitted now. > > Thanks. Now on to the wrong code issues. :-) > > Up to the change, the useless_type_conversion_p predicate was relying on > structural equivalence via the TYPE_CANONICAL check, now it only looks at the > outermost level (size, mode). Now some back-ends, most

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

> Added and comitted now. Thanks. Now on to the wrong code issues. :-) Up to the change, the useless_type_conversion_p predicate was relying on structural equivalence via the TYPE_CANONICAL check, now it only looks at the outermost level (size, mode). Now some back-ends, most notably x86-64,

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

On Fri, Oct 23, 2015 at 7:19 AM, Jan Hubicka wrote: > Hello, > this is a variant of patch I tested. After looking into the issue more, I > think we don't really need > to check types to be compatible (or we want to check it in other references, > too). It seems to me > that we

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

+ /* Changes in machine mode are never useless conversions unless. */ Unless what? Andreas. -- Andreas Schwab, SUSE Labs, sch...@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different."

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

Hello, this is a variant of patch I tested. After looking into the issue more, I think we don't really need to check types to be compatible (or we want to check it in other references, too). It seems to me that we should be able to drop /* Verify that access happens in similar

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

> > > > * tree.c (verify_type): Verify that TYPE_MODE match > > between TYPE_CANONICAL and type. > > * expr.c (store_expr_with_bounds): Revert my previous change. > > * expmed.c (store_bit_field_1): Revert prevoius change. > > * gimple-expr.c (useless_type_conversion_p):

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

> here is updated patch that applies changes suggested by Richard. I apologize > for the delay - the testing failed several times on gcc10.fsffrance.org for > me for out-of-memory errors (which are unrelated) and I was on the travel. > > Bootstrapped/regtested x86_64-linux, OK? > > *

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

On Wed, Oct 21, 2015 at 6:05 AM, Jan Hubicka wrote: > Hi, > here is updated patch that applies changes suggested by Richard. I apologize > for the delay - the testing failed several times on gcc10.fsffrance.org for me > for out-of-memory errors (which are unrelated) and I was on

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

> this is patch that reverts the TYPE_MODE mismatch related changes and > adds test to type checker that TYPE_MODE always match with TYPE_CANONICAL. > I have bootstrapped/regtested x86_64-linux, but unfrtunately the regtesting > had some unrelated noise (spawn failures). I am re-testing. I am on a

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

Hi, here is updated patch that applies changes suggested by Richard. I apologize for the delay - the testing failed several times on gcc10.fsffrance.org for me for out-of-memory errors (which are unrelated) and I was on the travel. Bootstrapped/regtested x86_64-linux, OK? * tree.c

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

> Why is Ada fliddling with the modes? Is it only for packed structures? Yes, in Ada packing or representation clauses are allowed to modify the type of components, so you can have e.g. a record type with size S1 and BLKmode and fields of this type with a packed version of this record type

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

> Adding back the mode check is fine if all types with the same TYPE_CANONICAL > have the same mode. Otherwise we'd regress here. It's true for the Ada compiler, the type fiddling machinery always resets it. -- Eric Botcazou

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

On Sun, Oct 18, 2015 at 7:14 PM, Jan Hubicka wrote: >> >> Adding back the mode check is fine if all types with the same TYPE_CANONICAL >> have the same mode. Otherwise we'd regress here. I thought we do for >> >> Struct x { int i; }; >> Typedef y x __attribute__((packed)); >>

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

Richard, I missed your reply earlier today. > > Therefore I would say that TYPE_CANONICAL determine mode modulo the fact > > that > > incoplete variant of a complete type will have VOIDmode instead of complete > > type's mode (during non-LTO). That is why I allow mode changes for casts > > from

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

> I was only tracking one isse I hit: Fortran/C interoperability nees LTO to > produce same TYPE_CANONICAl for signed and unsigned version of size_t. > Doing so broke useless_type_conversion because it used TYPE_CANONICAL. We > discussed the topic on the GNU Cauldron and decided that it is cleaner

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

Hello, > > I was only tracking one isse I hit: Fortran/C interoperability nees LTO to > > produce same TYPE_CANONICAl for signed and unsigned version of size_t. > > Doing so broke useless_type_conversion because it used TYPE_CANONICAL. We > > discussed the topic on the GNU Cauldron and decided

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

On October 18, 2015 6:06:51 PM GMT+02:00, Jan Hubicka wrote: >Hello, >> > I was only tracking one isse I hit: Fortran/C interoperability nees >LTO to >> > produce same TYPE_CANONICAl for signed and unsigned version of >size_t. >> > Doing so broke useless_type_conversion because it

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

> > Adding back the mode check is fine if all types with the same TYPE_CANONICAL > have the same mode. Otherwise we'd regress here. I thought we do for > > Struct x { int i; }; > Typedef y x __attribute__((packed)); > > And then doing > > X x; > Y y; > X = y; Do you have any idea how to

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

> >And AFAIK nobody answered the question: what do we gain by making this > >change? > >So far I have only seen breakages, suspicious fixes and code > >duplication... > > Honza wants the structural equality predicate (operand_equal_p) complete > (optimistically) for GIMPLE. There are two

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

> Well, it would (I think) ICE on assigning a packed variant to a non-packed > variant of a strict that happens to get a non-BLKmode when not packed. Is "it" GIMPLE here? My sentence was not very clear, I meant that I don't see why GIMPLE would have to care about whether there is a VCE or not

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

On October 17, 2015 11:26:43 AM GMT+02:00, Eric Botcazou wrote: >> Well, it would (I think) ICE on assigning a packed variant to a >non-packed >> variant of a strict that happens to get a non-BLKmode when not >packed. > >Is "it" GIMPLE here? My sentence was not very

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

On October 16, 2015 5:55:08 PM GMT+02:00, Eric Botcazou wrote: >> I wasn't aware that x86/IA-64 is still broken. I am flying to NY >tomorrow >> but will try to take a look. The ICEs are not caused by >operand_equal_p >> changes, but the change to useless_type_conversion

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

> I wasn't aware that x86/IA-64 is still broken. I am flying to NY tomorrow > but will try to take a look. The ICEs are not caused by operand_equal_p > changes, but the change to useless_type_conversion to ignore mode on > aggregate types. Sure, but I'd like to avoid hiding new problems against

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

> Eric, does that look ok WRT TYPE_ALIGN_OK? (that is, did we decide > TYPE_ALIGN_OK is no longer needed?) I'm not sure we need to care about TYPE_ALIGN_OK here so no objection by me. -- Eric Botcazou

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

On Wed, Oct 14, 2015 at 6:29 PM, Jan Hubicka wrote: > Hi, > this patch adds VIEW_CONVERT_EXPR which is another code omitted in > operand_equal_p. During bootstrap there are about 1000 matches. > > Bootstrapped/regtested x86_64-linux, OK? Eric, does that look ok WRT

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

> > I'm not sure we need to care about TYPE_ALIGN_OK here so no objection by me. > > Actually I have one: can we please fix the multiple Ada breakages caused by > the previous controversial change from Jan before going farther in the series? > The build is still broken on IA-64 and I'm still

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

> > * fold-const.c (operand_equal_p): Handle VIEW_CONVERT_EXPR. > > Index: fold-const.c > > === > > --- fold-const.c(revision 228735) > > +++ fold-const.c(working copy) > > @@ -2962,6 +2968,12 @@

Re: Add VIEW_CONVERT_EXPR to operand_equal_p

> I'm not sure we need to care about TYPE_ALIGN_OK here so no objection by me. Actually I have one: can we please fix the multiple Ada breakages caused by the previous controversial change from Jan before going farther in the series? The build is still broken on IA-64 and I'm still seeing ICEs

Add VIEW_CONVERT_EXPR to operand_equal_p

Hi, this patch adds VIEW_CONVERT_EXPR which is another code omitted in operand_equal_p. During bootstrap there are about 1000 matches. Bootstrapped/regtested x86_64-linux, OK? Honza * fold-const.c (operand_equal_p): Handle VIEW_CONVERT_EXPR. Index: fold-const.c