This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: just_select in combine.c:force_to_mode


On Fri, Dec 14, 2018 at 06:32:32AM +0000, SenthilKumar.Selvaraj@microchip.com wrote:
> Segher Boessenkool writes:
> > On Thu, Dec 13, 2018 at 09:39:52AM +0000, SenthilKumar.Selvaraj@microchip.com wrote:
> >>   When debugging PR 88253, I found that force_to_mode uses a parameter
> >>   (just_select) to prevent the function from returning a const0_rtx even
> >>   if none of the bits set by the rtx are needed. The comment says
> >>
> >>    "If JUST_SELECT is nonzero, don't optimize by noticing that bits in MASK
> >>    are all off in X.  This is used when X will be complemented, by either
> >>    NOT, NEG, or XOR."
> >>
> >>    and the code behaves the same way, but could someone help me
> >>    understand why?
> >
> > This was introduced in https://gcc.gnu.org/r6342 .
> 
> Yep :). Was hoping someone would have run into a similar situation,
> although trawling the gcc mailing lists didn't turn up anything useful.

The core problem is that nonzero_bits isn't symmetrical: it tells you
which bits are not guaranteed to be zero, but nothing about which bits
are one (other than that the bits that *are* zero are not one, of course).

> >>    This more complicated pattern doesn't match, and combine
> >>    moves on to another combination of insns, eventually resulting in PR 88253.
> >
> > Combine not doing some combination is never incorrect, of course (just not
> > what you want, possibly, but not the cause of a bug :-) )
> 
> Understood. The bug report said the PR does not show up for a bitwise
> OR, so just wanted to convey that this is where the divergence
> between the RTL for a bitwise OR vs XOR starts.

Ah, okay.

> >>    Isn't the simplification of
> >>
> >>    (subreg:QI (ior:HI (ashift:HI (zero_extend:HI (reg/v:QI 46 [ h ]))
> >>                 (const_int 8 [0x8]))
> >>             (reg:HI 55 [ D.1627 ]))
> >>
> >>    to
> >>
> >>    (subreg:QI (reg:HI 55 [ D.1627 ])
> >>
> >>    safe to do, wheter the outer insn code is XOR or IOR?
> >
> > Probably.
> 
> I was wondering if I should put out a patch that resets just_select for
> (the nested) IOR's operands, but maybe I should leave this untouched. As
> you said, this is more a missed optimization, and shouldn't really cause
> any bugs.

It seems to me that the latter expression always is a correct simplification,
so a patch to make that happen is welcome.  If it is simple enough it may
go in for GCC 9 still, otherwise, it will have to wait for GCC 10.

Thanks,


Segher


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]