This is an archive of the discontinued LLVM Phabricator instance.

[IPSCCP] Partial support for constrained intrinsics
AbandonedPublic

Authored by kpn on Dec 14 2021, 8:33 AM.

Details

Summary

This teaches the SCCP Solver about some of the constrained intrinsics by having them use the same codepaths as the non-constrained instructions.

Casting instructions along with the binary operators will come later.

Diff Detail

Event Timeline

kpn created this revision.Dec 14 2021, 8:33 AM
kpn requested review of this revision.Dec 14 2021, 8:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 14 2021, 8:33 AM
fhahn requested changes to this revision.Dec 14 2021, 8:42 AM
fhahn added inline comments.
llvm/lib/IR/Instruction.cpp
694

I think this change is strictly independent of supporting constrained intrinsics in SCCP and should be separate I think.

llvm/test/Transforms/SCCP/strictfp-float-nan-simplification.ll
1

could you preocmmit the tests?

This revision now requires changes to proceed.Dec 14 2021, 8:42 AM
nikic added inline comments.Dec 14 2021, 8:55 AM
llvm/lib/IR/Instruction.cpp
694

I think it would be better to address this by switching SCCP to use isInstructionTriviallyDead() instead. In fact, if we switch SCCP and FunctionSpecialization, we can drop this method entirely, as these seem to be the only users. Don't really see why we need a second, worse implementation if this functionality.

kpn updated this revision to Diff 437648.Jun 16 2022, 11:48 AM
kpn edited the summary of this revision. (Show Details)

Update for review comments. I've also reduced it down to just fcmp and fcmps having a functional impact. I don't have a good way to test the binary operators just yet.

Herald added a project: Restricted Project. · View Herald TranscriptJun 16 2022, 11:48 AM
nikic added a comment.Jun 28 2022, 9:31 AM

Why isn't this already handled by ConstantFoldCall?

kpn added a comment.Oct 3 2022, 11:49 AM

Why isn't this already handled by ConstantFoldCall?

Because ConstantFoldCall() is called from handleCallResult(), and we're skipping that part of handleCallBase() due to having constrained fcmp/fcmps behave just like the non-constrained. Instead we're constant folding in visitCmpInst().

Also, a constrained fcmp/fcmps is !isa<Function>, so while it is not overdefined, it does not fall into the block in markUsersAsChanged() that calls ConstantFoldCall().

I'll update the diff in a minute with the right set of tests...

kpn updated this revision to Diff 464756.Oct 3 2022, 11:51 AM

Rebase.

Update to use more targeted tests.

This still leaves fhahn's request not done because that's what D118392 is for.

nikic added a comment.Oct 3 2022, 11:58 AM

Why isn't this already handled by ConstantFoldCall?

Because ConstantFoldCall() is called from handleCallResult(), and we're skipping that part of handleCallBase() due to having constrained fcmp/fcmps behave just like the non-constrained. Instead we're constant folding in visitCmpInst().

Sorry, what I meant is: Why is this not handled by ConstantFoldCall() prior to your changes? There should be no need for special handling of these intrinsics here.

From a cursory look, I think the problem is that handleCallOverdefined() doesn't deal with metadata operands. I expect that if you fix this, then constrained intrinsics will "just work".

kpn abandoned this revision.Oct 24 2022, 8:45 AM

Abandoned in favor of D136466.