On Tue, Oct 20, 2020 at 01:55:56PM -0500, Segher Boessenkool wrote: > On Thu, Oct 08, 2020 at 09:27:54AM +1030, Alan Modra wrote: > > The existing "case AND" in this function is not sufficient for > > optabs.c:avoid_expensive_constant usage, where the AND is passed in > > outer_code. We'd like to cost AND of rs6000_is_valid_and_mask > > or rs6000_is_valid_2insn_and variety there, so that those masks aren't > > seen as expensive (ie. better to load to a reg then AND). > > > > * config/rs6000/rs6000.c (rs6000_rtx_costs): Combine CONST_INT > > AND handling with IOR/XOR. Move costing for AND with > > rs6000_is_valid_and_mask or rs6000_is_valid_2insn_and to > > CONST_INT. > > Sorry this took so long to review :-( > > On 64-bit BE this leads to *bigger* code, and closer observation shows > that some common sequences degrade on all configs. This seems to mostly > be about "andc" (and its dot form). It wasn't costed properly before, > but after your patch, a single instruction is replaced by three. > > Could you look into this?
~/build/gcc-alan/gcc$ for z in *.o; do if test `objdump -dr $z | grep andc | wc -l` != `objdump -dr ../../gcc/gcc/$z | grep andc | wc -l`; then echo $z; fi; done gimplify.o insn-emit.o insn-opinit.o insn-recog.o rs6000-string.o All of these are exactly the case I talked about in https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553919.html "Sometimes correct insn cost leads to unexpected results. For example: extern unsigned bar (void); unsigned f1 (unsigned a) { if ((a & 0x01000200) == 0x01000200) return bar (); return 0; } emits for a & 0x01000200 (set (reg) (and (reg) (const_int 0x01000200))) at expand time (two rlwinm insns) rather than the older (set (reg) (const_int 0x01000200)) (set (reg) (and (reg) (reg))) which is three insns. However, since 0x01000200 is needed later the older code after optimisation is smaller." Things have changed slightly since I wrote the above, with the two rlwinm insns being emitted at expand time, so you see (set (reg) (and (reg) (const_int 0xffffffffff0003ff))) (set (reg) (and (reg) (const_int 0x0000000001fffe00))) but of course that doesn't change anything regarding the cost of "a & 0x01000200". -- Alan Modra Australia Development Lab, IBM
