[PATCH] D106447: [DAGCombine] DAGTypeLegalizer::GenWidenVectorLoads(): make use of dereferenceability knowledge

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/test/CodeGen/AMDGPU/kernel-args.ll:457-458
 ; EGCM-DAG: T{{[0-9]\.[XYZW]}}, KC0[8].Z
-; SI-DAG: s_load_dwordx8 s{{\[[0-9]:[0-9]+\]}}, s[0:1], 0x19
-; SI-DAG: s_load_dwordx2 s{{\[[0-9]:[0-9]+\]}}, s[0:1], 0x21
-; MESA-VI-DAG: s_load_dwordx8 s{{\[[0-9]:[0-9]+\]}}, s[0:1], 0x64
-; MESA-VI-DAG: s_load_dwordx2 s{{\[[0-9]:[0-9]+\]}}, s[0:1], 0x84
-; HSA-GFX9-DAG: s_load_dwordx8 s[{{[0-9]+:[0-9]+}}], s[4:5], 0x40
-; HSA-GFX9-DAG: s_load_dwordx4 s[{{[0-9]+:[0-9]+}}], s[4:5], 0x60
+; SI-DAG: s_load_dwordx16 s{{\[[0-9]:[0-9]+\]}}, s[0:1], 0x19
+; SI-DAG: s_load_dwordx2 s{{\[[0-9]:[0-9]+\]}}, s[0:1], 0x9
+; MESA-VI-DAG: s_load_dwordx16 s{{\[[0-9]:[0-9]+\]}}, s[0:1], 0x64
----------------
lebedev.ri wrote:
> lebedev.ri wrote:
> > foad wrote:
> > > I think this diff is misleading. I've pushed an alternative version here, which is based on first autogenerating the checks in this file before applying D106447: https://github.com/jayfoad/llvm-project/commit/ed4a53da7c265675510caeb2c773f1b461354347
> > > 
> > > In this particular case (v5i64_arg for SI) the relevant part of the diff is:
> > > ```
> > > -; SI-NEXT:    s_load_dwordx8 s[4:11], s[0:1], 0x19
> > > -; SI-NEXT:    s_load_dwordx2 s[12:13], s[0:1], 0x9
> > > -; SI-NEXT:    s_load_dwordx2 s[0:1], s[0:1], 0x21
> > > -; SI-NEXT:    s_mov_b32 s15, 0xf000
> > > -; SI-NEXT:    s_mov_b32 s14, -1
> > > +; SI-NEXT:    s_load_dwordx16 s[4:19], s[0:1], 0x19
> > > +; SI-NEXT:    s_load_dwordx2 s[0:1], s[0:1], 0x9
> > > +; SI-NEXT:    s_mov_b32 s3, 0xf000
> > > +; SI-NEXT:    s_mov_b32 s2, -1
> > > ```
> > > 
> > > I.e. the load from scaled offset 0x9 is untouched; the significant difference is that the dwordx8 and dwordx2 load from offsets 0x19 and 0x21 have been replaced by a single dwordx16 load from offset 0x19.
> > That's why nobody likes manually-written check lines..
> > Let me rebase this..
> Actually, i see that you have not pushed https://github.com/llvm/llvm-project/commit/00b0f1ed7d5833695ba266854ee1d07467ddcf9c, so there isn't much point in rebasing.
> Do you want to push that one?
> 
Pushed for review as D107052 because I had to make some non-trivial changes to the RUN lines.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106447/new/

https://reviews.llvm.org/D106447