[PATCH] D90286: [Analysis] Improve EmitGEPOffset by avoiding summ with zero

Johannes Doerfert via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 29 09:02:08 PDT 2020


jdoerfert requested changes to this revision.
jdoerfert added a comment.
This revision now requires changes to proceed.

> and there is a related test (test/Transforms/InstCombine/sub-gep.ll), which I believe is enough for this small improvement.

Is this test changed? It seems to be missing from the diff.

> Keep in mind that EmitGEPOffset() could be used in other custom passes that might not require subsequent instcombine to run.

Noted.

> I believe we do not have to adjust sources to make them look better being formatted by the clang formatter. We should improve the formatter instead. But this should not prevent this patch to be accepted. I'm ok to rename the variables for a better look if this is the only thing that is needed for LGTM, though.

Let me rephrase. Please avoid assignments over 3-5 lines, it is hard to read.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90286



More information about the llvm-commits mailing list