[PATCH] D104282: [FuncSpec] Use std::pow instead of operator^

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu added inline comments.


================
Comment at: llvm/test/Transforms/FunctionSpecialization/function-specialization-loop.ll:2
+; RUN: opt -function-specialization -func-specialization-avg-iters-cost=3 -S < %s | FileCheck %s
+
+; check the bonus for specializing constant arguments are powered by the depth of the loop.
----------------
SjoerdMeijer wrote:
> Just checking, this test didn't trigger before, and now it does? 
> If so, would be good to precommit the test, then we can also see the differences better here.
Yeah, I had checked that.


================
Comment at: llvm/test/Transforms/FunctionSpecialization/function-specialization-recursive.ll:41-42
 define i32 @main() {
 ; CHECK-LABEL: @main(
-; CHECK-NEXT:    [[TEMP_I:%.*]] = alloca i32, align 4
-; CHECK-NEXT:    [[TMP1:%.*]] = bitcast i32* [[TEMP_I]] to i8*
-; CHECK-NEXT:    call void @llvm.lifetime.start.p0i8(i64 4, i8* nonnull [[TMP1]])
-; CHECK-NEXT:    call void @print_val(i32 1)
-; CHECK-NEXT:    store i32 2, i32* [[TEMP_I]], align 4
-; CHECK-NEXT:    call void @recursiveFunc(i32* nonnull [[TEMP_I]])
-; CHECK-NEXT:    [[TMP2:%.*]] = bitcast i32* [[TEMP_I]] to i8*
-; CHECK-NEXT:    call void @llvm.lifetime.end.p0i8(i64 4, i8* nonnull [[TMP2]])
+; CHECK-NEXT:    call void @recursiveFunc(i32* nonnull @Global)
 ; CHECK-NEXT:    ret i32 0
----------------
SjoerdMeijer wrote:
> ChuanqiXu wrote:
> > Now it couldn't specialize recursiveFunc due to the change of cost model. I would try to fix it by considering branch condition in getUserBonus.
> I don't think it was able to specialise recursive (see also comment at the top). 
> I haven't looked very long at this, but something else must have led to different codegen.
Yeah, I think the title is confusing. Function Specialization only did simple specialization without considering recursiveness (I don't know the noun form). Then the change in this test case looks good to me.


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

https://reviews.llvm.org/D104282