[PATCH] D77160: Fix PR45371: SeparateConstOffsetFromGEP clean up bookkeeping

Jonathan Roelofs via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 31 15:27:40 PDT 2020


jroelofs marked 2 inline comments as done.
jroelofs added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp:580
+
+  if (ConstantOffset == 0) {
+    // Reset the chain again, since exploring the RHS didn't work out either.
----------------
bjope wrote:
> Coding style: You may skip the braces here.
Haven't seen (m)any braceless if's with a comment inside them. Happy to reword and drop them though.


================
Comment at: llvm/test/Transforms/SeparateConstOffsetFromGEP/pr45371.ll:6
+
+define void @PR45371() {
+; CHECK-LABEL: @PR45371(
----------------
ajwock wrote:
> Can we have a name for this file and this function that's a little more self explanatory?  If it weren't attached to this commit I would have no idea what's being tested.
It's not uncommon to name tests after their corresponding bugzilla number:

```
$ find llvm/test/ -name "pr[0-9]*.ll" | wc -l
     791
```

Suggestions on a better function name?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77160





More information about the llvm-commits mailing list