This is an archive of the discontinued LLVM Phabricator instance.

[ConstantFold] Fold the comparison of bitcasted global values
ClosedPublic

Authored by scui on Sep 17 2020, 12:24 PM.

Details

Summary

This is to simplify icmp instructions in the form like:

%cmp = icmp eq i32 (i8*, i8*)* bitcast (i32 (i32**, i32**)* @f32 to i32 (i8*, i8*)), bitcast (i32 (i64**, i64**) @f64 to i32 (i8*, i8*)*)

Here @f32 and @f64 are two functions.

Diff Detail

Event Timeline

scui created this revision.Sep 17 2020, 12:24 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 17 2020, 12:24 PM
scui requested review of this revision.Sep 17 2020, 12:24 PM
scui updated this revision to Diff 292817.Sep 18 2020, 9:04 AM

Revert 2004-11-27-SetCCForCastLargerAndConstant.ll and PR17073.ll

scui updated this revision to Diff 292825.Sep 18 2020, 9:22 AM

Fix previous reverion of 2004-11-27-SetCCForCastLargerAndConstant.ll and PR17073.ll

jsji retitled this revision from Fold the comparison of bitcasted global values to [ConstantFold] Fold the comparison of bitcasted global values .
scui added a comment.Sep 30 2020, 7:47 AM

Anyone has any comments for this patch? Any comments or suggestions are appreciated.

efriedma added inline comments.
llvm/lib/IR/ConstantFold.cpp
1759

I guess this handles the case where both operands are bitcasts due to other folds?

Is it possible to end up in a situation where one operand is a bitcast, and the other is a GEP? I guess we could leave that as an area for future improvement.

llvm/test/Transforms/InstCombine/pr32686.ll
12

I think this test needs a constant expression that won't fold to exercise the right codepath; please adjust the expression so it doesn't fold.

Same applies to the SCCP tests.

scui added inline comments.Oct 5 2020, 10:11 AM
llvm/lib/IR/ConstantFold.cpp
1759

I guess this handles the case where both operands are bitcasts due to other folds?

Yes, that's right.

Is it possible to end up in a situation where one operand is a bitcast, and the other is a GEP? I guess we could leave that as an area for future improvement.

Yes. I'll leave this for the future improvement.

llvm/test/Transforms/InstCombine/pr32686.ll
12

Thanks for the suggestion. I'll make the variables common.

scui updated this revision to Diff 296219.Oct 5 2020, 10:15 AM

Adjust the test cases.

efriedma added inline comments.Oct 5 2020, 10:31 AM
llvm/test/Transforms/InstCombine/pr32686.ll
30

The "extra" version of this testcase isn't really helpful in this context; the version that folds will be folded away before instcombine can see it, so it's only testing constant folding itself.

If you want test coverage for additional constant expressions, please add them to the icmp folding test.

scui updated this revision to Diff 296246.Oct 5 2020, 11:36 AM

Remove the added extra cases from the existing test files

scui added inline comments.Oct 5 2020, 11:39 AM
llvm/test/Transforms/InstCombine/pr32686.ll
30

The added test cases in this file and the sccp tests are removed. One extra test case is added in the new test file.

scui edited reviewers, added: efriedma; removed: eli.friedman.Oct 6 2020, 11:34 AM
efriedma accepted this revision.Oct 9 2020, 2:04 PM

LGTM

Do you have commit access? If not, I can commit it for you; what should I put in the git "Author" line?

This revision is now accepted and ready to land.Oct 9 2020, 2:04 PM
scui added a comment.Oct 9 2020, 4:12 PM

LGTM

Do you have commit access? If not, I can commit it for you; what should I put in the git "Author" line?

Thanks. I don't have commit access. The author line should be --author="Shimin Cui <scui@ca.ibm.com>".

This revision was automatically updated to reflect the committed changes.