[PATCH] D74444: [NVPTX, LSV] Move the LSV optimization pass to later when the graph is cleaner

[Artem Belevich via Phabricator via llvm-commits]

tra added inline comments.


================
Comment at: llvm/test/CodeGen/NVPTX/vector-loads-complex.ll:12
+
+define void @reduce_10(i8* nocapture readonly align 16 dereferenceable(134217728) %alloc0, i8* align 64 dereferenceable(1024) %alloc1) local_unnamed_addr #0 {
+entry:
----------------
Couple of general notes:

I'd attempt to reduce IR to the bare minimum necessary to demonstrate/test intended behavior. This IR is not bad, but I think it may be reduced further to the point where we end up with just one vectorized 2-element load.

It's a good practice to 'anchor the checks with fixed start/end points
```
CHECK-LABEL: <function_name>

... # other checks on whatever happens inside the function.

CHECK: ret
``` 

It helps to delineate the search boundaries for FileCheck. Not a big deal now, as you only have one function, but I would not be surprised if more tests would be added over time.



================
Comment at: llvm/test/CodeGen/NVPTX/vector-loads-complex.ll:14-18
+; CHECK: ld.global.v2.u8
+; CHECK: ld.global.v2.u8
+; CHECK: ld.global.v2.u8
+; CHECK: ld.global.v2.u8
+; CHECK-NOT: ld.global
----------------
Negative matching is tricky. As written, FileCheck will only ensure that there's no ld.global between the fourth instance of ld.global.v2.u8 and the end of the file.

I'd start by attempting to place the checks close to the `load` statements they coalesce into. It's not always possible, but it probably works in this case.

As long as the number of loads is fixed/predictable, positively matching all of them makes negative matching unnecessary, so I'd just drop it.


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

https://reviews.llvm.org/D74444