[PATCH] D93694: [MachineLICM] SinkIntoLoop: analyse successive store instructions

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineLICM.cpp:817
+
+    if (IsLoopInvariantInst(*I, HasSuccessiveStoreInst(*I)) &&
+        !HasLoopPHIUse(&*I)) {
----------------
This only checks for stores/calls in the preheader. What about the case where there are stores/clobbering calls in the loop body,

e.g. something like 

```
    %20:gpr64common = ADRP target-flags(aarch64-page) @A
    STRWui %3, %20, target-flags(aarch64-pageoff, aarch64-nc) @A  :: (store 4 into `i32* getelementptr inbounds ([100 x i32], [100 x i32]* @A, i64 0, i64 0)`)
```

in the test case?




================
Comment at: llvm/test/CodeGen/AArch64/machine-licm-sink-instr.mir:159
   ; CHECK:   $w0 = COPY [[COPY1]]
   ; CHECK:   BL @_Z3usei, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit $w0, implicit-def $sp, implicit-def $w0
   ; CHECK:   ADJCALLSTACKUP 0, 0, implicit-def dead $sp, implicit $sp
----------------
I don't think there's anything preventing `_Z3usei` to modify `@A` in the loop, so if we move the load into the loop, we may read different values on each iteration?


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

https://reviews.llvm.org/D93694