[PATCH] D28693: [InlineCost] Use TTI to check if GEP is free.

[Chad Rosier via Phabricator via llvm-commits]

mcrosier added inline comments.


================
Comment at: test/Transforms/Inline/gep-cost.ll:1
+; RUN: opt -inline < %s -S -debug-only=inline-cost 2>&1 | FileCheck %s
+
----------------
haicheng wrote:
> mcrosier wrote:
> > I apologize for my lack of understanding.  Assuming this isn't obvious to someone more familiar with the inliner cost model, would you mind adding a few comments to this test case explaining exactly what you're testing?
> > 
> > I diffed the output from this test with and without your change and the only cost that was changed was for inner1.  Thus, I'm trying to understand the significance of inner2.
> Thank you, Chad.  I will add appropriate comments.
> 
> The GEP in inner2() is reg+imm+reg which is not legal addressing mode for AArch64.  I expect to see only ret  can be simplified.  The current implementation only considers GEP with all constant indices are foldable, so the GEP in inner2() is considered not foldable even without my patch.  I add this test for completeness.
> 
> The GEP in inner1(), however, is reg+reg and is legal for AArch64.  It can be detected only with my patch.
Ah, I see.  Suggestions below.


================
Comment at: test/Transforms/Inline/gep-cost.ll:11
+}
+; CHECK: Analyzing call of inner1
+; CHECK: NumInstructionsSimplified: 2
----------------
Here you could probably say something like:

  // The GEP in inner1() is [insert legal addressing mode], which is a legal addressing mode for AArch64.  Thus, both the gep and ret can be simplified.


================
Comment at: test/Transforms/Inline/gep-cost.ll:19
+
+; CHECK: Analyzing call of inner2
+; CHECK: NumInstructionsSimplified: 1
----------------
Here you could probably say something like:

  // The GEP in inner2() is  reg+imm+reg, which is not a legal addressing mode for AArch64.  Thus, only the ret can be simplified and not the gep.


Repository:
  rL LLVM

https://reviews.llvm.org/D28693