This is the mail archive of the
gcc@gcc.gnu.org
mailing list for the GCC project.
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