Page MenuHomePhabricator

[LSR][TTI][RISCV] Add isAllowTerminatingConditionFoldingAfterLSR into TTI and enable it for RISC-V
Needs ReviewPublic

Authored by eopXD on Sep 29 2022, 9:39 AM.

Details

Summary

Previously in D132443, the transformation was added and guarded by an option.
This commit attempts to create an TTI and enable it for the RISC-V backend.

Diff Detail

Event Timeline

eopXD created this revision.Sep 29 2022, 9:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 29 2022, 9:39 AM
eopXD requested review of this revision.Sep 29 2022, 9:39 AM
eopXD added a comment.EditedSep 29 2022, 9:41 AM

I ran through lit test under llvm/test/CodeGen/RISCV before submitting this patch, these two affected test case shows the benefit of the transformation.

JojoR added a subscriber: JojoR.Oct 11 2022, 1:53 AM

There is a error report as below if i apply this patch on branch main 8d4ebd1a7c9e1f47a4a610aeb41d1613f822ee20
error: Terminating value is not safe to expand, need to add it to predicate.

my compiler option is "-march=rv64gc -O2"
my test case is :

eopXD added a comment.Oct 11 2022, 4:15 AM

@JojoR Thanks for the report. I was wondering that whether the assertion would hit. Will add a patch to fix this.

JojoR added a comment.Oct 13 2022, 2:09 AM

I have a question about patch D132443, why do not you put optimization in pass "iv-users" ? I think it belongs to IV analyzation.

because that the pass "iv-users" do analyzation only without any transformation ?

eopXD added a comment.Oct 13 2022, 2:14 AM

I have a question about patch D132443, why do not you put optimization in pass "iv-users" ? I think it belongs to IV analyzation.

because that the pass "iv-users" do analyzation only without any transformation ?

IVUser is an analysis pass, and the LSR pass uses it before doing strength reduction.
This transformation is beneficial after LSR, not before it.

JojoR added a comment.Oct 13 2022, 2:26 AM

I have a question about patch D132443, why do not you put optimization in pass "iv-users" ? I think it belongs to IV analyzation.

because that the pass "iv-users" do analyzation only without any transformation ?

IVUser is an analysis pass, and the LSR pass uses it before doing strength reduction.
This transformation is beneficial after LSR, not before it.

Got it, thanks :)

JojoR added a comment.Oct 13 2022, 2:49 AM

@eopXD I saw the patch D132443 only support eq/neq condterm, and I want to add gt/lt,
any suggestion for me ? or you have plan to implement that ?

eopXD added a comment.Oct 13 2022, 4:28 AM

@eopXD I saw the patch D132443 only support eq/neq condterm, and I want to add gt/lt,
any suggestion for me ? or you have plan to implement that ?

I will submit proceeding patches for more support, thank you.

eopXD added a comment.Oct 20 2022, 8:30 PM

There is a error report as below if i apply this patch on branch main 8d4ebd1a7c9e1f47a4a610aeb41d1613f822ee20
error: Terminating value is not safe to expand, need to add it to predicate.

my compiler option is "-march=rv64gc -O2"
my test case is :

Is it possible for you to have a reduced test case on this? It would help to land the revision to fix this. Thank you.

eopXD updated this revision to Diff 469899.Oct 22 2022, 7:52 AM

Rebase to latest main