[PATCH] D137019: [InstCombine] refactor the SimplifyUsingDistributiveLaws NFC

Allen zhong via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Oct 29 19:14:41 PDT 2022


Allen marked 3 inline comments as done.
Allen added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:733
 
+Value *InstCombinerImpl::SimplifyFactorization(BinaryOperator &I) {
+  Value *LHS = I.getOperand(0), *RHS = I.getOperand(1);
----------------
spatel wrote:
> The current formatting rules say function names begin with a lower-case letter.
> I'd prefer not to use the term "Simplify" here because that implies that we are not creating new values (InstSimplify). 
> Use "doFactorization()" or "tryFactorizationFolds" or some variant of that?
Done, thanks 


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:776
 /// Returns the simplified value, or null if it didn't simplify.
 Value *InstCombinerImpl::SimplifyUsingDistributiveLaws(BinaryOperator &I) {
   Value *LHS = I.getOperand(0), *RHS = I.getOperand(1);
----------------
spatel wrote:
> Since we're making changes here, I'd rename this to meet current formatting guidelines.
> "foldUsingDistributiveLaws()" ?
Apply your comment, thanks


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

https://reviews.llvm.org/D137019



More information about the llvm-commits mailing list