[PATCH] D26848: [PATCH] Reduce inline thresholds to compensate for cost changes

[James Molloy via llvm-commits]

jmolloy added a comment.

Hi Michael,

Sorry for the lack of context - see inline comments.

Cheers,

James



================
Comment at: test/Transforms/Inline/ephemeral.ll:24
 ; CHECK-NOT: call i1 @inner
-define i1 @outer() optsize {
-   %r = call i1 @inner()
-   ret i1 %r
+define i32 @outer() optsize {
+   %r = call i32 @inner()
----------------
mzolotukhin wrote:
> Why is this change needed?
Simply, the inner function was doing too much work. It was inlined in minsize mode before, but isn't now (and the new behaviour is correct - the inlining would previously have increased codesize).

Reducing the amount of work the inner function does allows it to be inlined.


================
Comment at: test/Transforms/Inline/inline-fp.ll:135-136
 
-attributes #0 = { minsize optsize }
-attributes #1 = { minsize optsize "use-soft-float"="true" }
+attributes #0 = { optsize }
+attributes #1 = { optsize "use-soft-float"="true" }
----------------
mzolotukhin wrote:
> Is this one of the cases where we can not preserve the original behavior, or we intentionally want another one here (no context in the patch makes it harder to comprehend from here)?
Apologies for the lack of context - error on my side.

The testcase is attempting to ensure that soft-float calls are more expensive to the inliner than hardware floating point ops. The test was obviously reduced from a real program, and expects an inner function to be inlined.

This inner function doesn't contain *any* function calls, and is the maximum size we would have inlined previously. We've reduced the minsize threshold, so these should no longer get inlined.

Upping the test to requiring optsize instead of minsize changes the threshold, and the original behaviour is maintained.


Repository:
  rL LLVM

https://reviews.llvm.org/D26848