[PATCH 2/5] R600: Use SIGN_EXTEND_INREG for SEXT loads

Matt Arsenault arsenm2 at gmail.com
Tue May 26 13:56:27 PDT 2015


> On May 26, 2015, at 1:41 PM, Jan Vesely <jan.vesely at rutgers.edu> wrote:
> 
> On Mon, 2015-05-25 at 18:58 -0700, Matt Arsenault wrote:
>>> On May 25, 2015, at 5:08 PM, Jan Vesely <jan.vesely at rutgers.edu> wrote:
>>> 
>>> ---
>>> 
>>> Probably just a missed optimization somewhere, nevertheless SIGN_EXTEND
>>> is more intuitive in this situation
>>> 
>>> lib/Target/R600/R600ISelLowering.cpp |   9 +--
>>> test/CodeGen/R600/load.ll            | 109 +++++++++++++----------------------
>>> 2 files changed, 42 insertions(+), 76 deletions(-)
>> 
>> LGTM. I’m surprised this wasn’t folded into this already requiring test changes
> 
> thanks, I pushed the first two. It was a bit surprising for me too
> because whenever I test:
> 
>  %1 = load i32, i32 addrspace(X)* %in
>  %2 = shl i32 %1, 24
>  %3 = ashr i32 %2, 24
>  store i32 %3, i32 addrspace(1)* %out
>  ret void
> 
> For X = 0,1,2,3
> 
> I see BFE_INT.
> Could it be that the transformation for some reason is run before the
> load gets lowered Is it worth investigating?

The DAGCombiner should run one final time after the DAG is legalized and catch this, so it should be happening. It might be useful to find out why, but I wouldn’t spend too much time debugging it.



More information about the llvm-commits mailing list