[PATCH] D90328: Eliminates dead store of an exisiting value

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:2395
+      MemoryAccess *DefAccess = dyn_cast<MemoryAccess>(Def);
+      if (DefAccess == NULL)
+        continue;
----------------
It is common to avoid compares with `NULL/nullptr` and instead use `if (!DefAccess)`. Also, the condition can be combined with the one below?


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:2407
+          MemoryAccess *UseDefAccess = UseDef->getDefiningAccess();
+          if (UseDefAccess == NULL) {
+            continue;
----------------
Style guide suggest to not use `{}` for single line bodies.


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:2410
+          }
+          Instruction *UseInst = cast<MemoryUseOrDef>(UseDefAccess)->getMemoryInst();
+          if (UseInst == NULL) {
----------------
You need to check if UseDefAccess is actually a use or def. It could be a MemoryPhi.

You could use something like `MemoryAccess *UseDefAccess = dyn_cast_or_null<MemoryUseOrDef>(UseDef->getDefiningAccess())`

Test case
```
define void @main(i32* %a, i32* %b, i1 %c) {
entry:
  br label %for.body

for.body:                                         ; preds = %if.end8, %entry
  br i1 %c, label %exit, label %loop.latch

loop.latch:                                          ; preds = %for.body
  store i32 0 , i32* %a
  store i32 1, i32* %b
  br label %for.body

exit:                                         ; preds = %for.body
  call void @foo()
  ret void
}

declare void @foo()
```


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

https://reviews.llvm.org/D90328