Re: [RS6000] Fix PR61098, Poor code setting count register

[Alan Modra <amodra@gmail.com>]

Hi David,
On Tue, May 13, 2014 at 11:46:20PM -0400, David Edelsohn wrote:
> Danny may have re-organized the code, but I thought that it originally
> came from Tom Rixx, if not earlier.

OK, I'm not trying to apportion blame.  My name is on plenty of dodgy
code in the rs6000 backend too.  :)

> I seem to remember problems in the past with late creation of TOC
> entries for constants causing problems, so it was easier to fall back
> to materializing all integer constants inline. I don't remember the
> PRs, but I think there were issues with creating a TOC if the late
> constant were the only TOC reference, or maybe the issue was buying a
> stack frame to materialize the TOC/GOT for a late constant.  And
> maximum 5 instruction sequence is not really bad relative to a load
> from the TOC (even with medium model / data in TOC). There are a lot
> of trade-offs with respect to I$ expansion versus the load hitting in
> the L1 D$.

Sure, but Steve will tell you that the 5 instruction sequence is both
slower due to all the dependent ops, and results in larger code+data
size.  We definitely want to avoid it if possible, and pr67836 shows a
case taken from glibc math library code where there should be no
problem in using the TOC.

> Alpha emit_set_const does limit the number of instructions, but that
> is a search for a more efficient sequence than the naive sequence. The
> Alpha splitters use alpha_split_const_mov(), which tries
> alpha_emit_set_const() for an efficient sequence and then falls back
> to alpha_emit_set_long_const() for a naive sequence.  Alpha uses PLUS
> instead of IOR because of the way its ISA works.
> alpha_emit_set_long_const() always will materialize the constant and
> does not check for a maximum number of instructions. This is why it's
> comment says "fall back to straight forward decomposition".
> 
> However, alpha_emit_set_long_const() and alpha_split_const_mov() can
> fail, presumably because emit_move_insn() fails, not because of
> reaching a maximum number of instructions.
> 
> alpha_legitimate_constant_p() rejecs expensive constants early. Once
> the splitter is invoked, it always tries to materialize the constant,
> but the splitter apparently can fail for other reasons.

No, that is wrong.  alpha_emit_set_const does *not* always try to
materialize the constant inline.  It does so for constants that need
more than three instructions only when TARGET_BUILD_CONSTANTS.

> I don't mind exploring the benefits of tighening up
> rs6000_legitimate_const(), but I'm not sure it's an obvious win,
> especially with the potential failure corner cases.

Yes, those potential corner cases have me worried too..

> However, I want to have a better understanding about the part of the
> patch that removes the FAIL path from the splitters.

That part was really quite simple.  I was removing dead code.
rs6000_emit_set_const has never returned anything but DEST, right from
the initial commit.  It can't be called with DEST == NULL, so
"dest = gen_reg_rtx (mode)" is also dead code.

However, I retracted that patch because I now think
rs6000_emit_set_const should in fact sometimes result in the splitter
failing, exactly as is done in the alpha port.

-- 
Alan Modra
Australia Development Lab, IBM