Hi Jim,

On Thu, Sep 05, 2019 at 04:40:56PM -0700, Jim Wilson wrote:
> On Thu, Sep 5, 2019 at 3:03 PM Segher Boessenkool
> <seg...@kernel.crashing.org> wrote:
> > My big question is why do other targets not have this problem?  Or what
> > is it they do differently?
> 
> RISC-V doesn't have convenient sign/zero extend instructions.  We also
> have a 12-bit immediate, whereas most other RISCs have a 16-bit
> immediate, so we can't easily load a constant and mask to zero-extend
> an HImode value.  Thus we have a fair amount of splitters that emit
> two shifts for zero/sign extend operations, or try to optimize various
> combinations of shifts or AND operations that can map to instructions
> that we do have.

There are other targets in that same boat, it's not *so* unusual.

> We also have two splitters that were reusing input
> registers for intermediate values when they should have been using a
> clobber to get a temporary.

Ah, that is just it I guess.  And the problem happens with *pseudos*,
which have just their one mode always (but can be used in another mode).
And then things go wrong if you use one in a bigger mode than it is.

Hrm I wonder if we could (or should) protect against this kind of thing
in generic code.  combine could have seen you did something undefined I
suppose.

> The failure case is when we end up
> generating something like (lshiftrt:DI (subreg:DI (reg:SI)) (const_int
> 48)) after a 48 bit left shift with a paradoxical reg input reused for
> an intermediate result, and the rtl simplifier says that must be zero
> because an SImode reg doesn't have any valid bits in the upper
> 32-bits.

Since it's a pseudo, not a hard reg, yeah.

> This is may also be tied into the fact that we have
> WORD_REGISTER_OPERATIONS defined.  Some funny stuff can happen with
> that defined, and I think most of the other major ports no longer
> define it.

Yeah.  Not only because of problems, but also because it doesn't help
much, and hurts in cases too, so it depends what way the balance swings
for your target.

> I haven't yet been tempted enough to try to remove that
> define from the RISC-V port yet, but suspect it will eventually be
> necessary.  I think the WORD_REGISTER_OPERATIONS define is so badly
> misused by the optimizer nowadays that it isn't worth the trouble
> anymore.  But removing it will be a lot of work to check for
> performance and code size regressions, so I haven't looked at it yet,
> and may not for a long while.

I tested it yesterday with builds of Linux.  Compiler from Sunday,
kernel tree a few weeks old.  riscv32 and riscv64, both with their
defconfigs.  Removing WORD_REGISTER_OPERATIONS results in 0.001%
smaller code for riscv32, and 0.036% larger code for riscv64.  For
the latter, shift/shift combos were often involved.  But it is
pretty minimal either way :-)

> The other three patterns that were fixed by adding a check for
> paradoxical regs have no known failure case, and were only fixed for
> completeness.  These are hypothetically necessary changes, and may not
> in fact be actually necessary, but they appear to be completely
> harmless so I saw no harm in adding them.  In the testing I did, I
> never saw a paradoxical reg here.  But that isn't proof that a
> paradoxical reg can never occur here.

Thanks for the explanations, that clears things up :-)


Segher

Reply via email to