[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