[LLVMdev] Avoiding load narrowing in DAGCombiner

[Matt Johnson]

Hi Eli,

On 07/27/2011 04:59 PM, Eli Friedman wrote:
> 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?

Perfect!  I'm using 2.8 for now (am hoping to roll forward to trunk and 
stay there in a month or two), and 2.8 only had that check for 
SIGN_EXTEND_INREG, not SRL or others.  The bugfix was in r124587.

I'm now seeing a similar problem with SimplifyDemandedBits() on a 
4-byte-aligned i8 load.  LowerLOAD() emits an aligned LD4 followed by an 
AND with constant 0xFF.  SimplifyDemandedBits() sees this and changes it 
to a zero-extended LD1.  Is this the same situation, where a bugfix was 
made after 2.8?  Any idea where to look?

>
> -Eli
Thanks!
-Matt