[LLVMdev] Avoiding load narrowing in DAGCombiner

[Eli Friedman]

On Wed, Jul 27, 2011 at 2:28 PM, Matt Johnson
<johnso87 at crhc.illinois.edu> wrote:
> Hi All,
>     I'm writing a backend for a target which only supports 4-byte,
> 4-byte-aligned loads and stores.  I custom-lower all {*EXT}LOAD and
> STORE nodes in TargetISelLowering.cpp to take advantage of all alignment
> information available to the backend, rather than treat each load and
> store conservatively, which takes O(10) instructions.  My target's
> allowsUnalignedMemoryOperations() always returns 'false', and the
> setOperationAction()s for i8,i16,i32 loads and stores are all 'Custom'.
>
>     I'm running into a problem where DAGCombiner is being too clever
> for me; it runs LegalizeDAG, which calls my custom LowerLOAD() and
> LowerSTORE() routines (which emit between 1 and O(10) SDValues,
> depending on alignment information), and then runs DAGCombine.  To lower
> an i16 STORE that is known to be in the high-addressed 2 bytes of a word
> on my little-endian target, I emit and LD4 from the word-aligned address
> and an SRL 16 to shift the i16 into the LSbits of the register.
>
> DAGCombine visit()s an ISD::SRL node and notices that it is
> right-shifting the result of an LD4 from %arrayidx4 by 16 bits, and
> replaces it with an LD2 from %arrayidx+2.
>
> Replaces
> --------
> 0x17f7070: i32,ch = load 0x17faa00, 0x17f7f70, 0x17f6a70<LD4[%arrayidx4]>
> 0x17f94c0: i32 = Constant<16> [ORD=9] [ID=10]
> 0x17f7470: i32 = srl 0x17f7070, 0x17f94c0
>
> With
> ----
> 0x17fceb0: i32,ch = load 0x17faa00, 0x17fac00,
> 0x17f6a70<LD2[%arrayidx4+2], zext from i16>
>
> That seems like a logical thing to do on a lot of targets, but in my
> case, the LD4->SRL combo was emitted precisely because I *don't* want a
> ZEXTLOAD i16.  I'm wondering if there is:
>
> a) A way to turn off this optimization in DAGCombine if your target
> doesn't support LD2 natively

This.

I think you're talking about DAGCombiner::ReduceLoadWidth... and the
"if (LegalOperations && !TLI.isLoadExtLegal(ExtType, ExtVT))" is
supposed to ensure that the transformation in question is safe.  That
said, IIRC I fixed a bug there recently; are you using trunk?

-Eli