-
Notifications
You must be signed in to change notification settings - Fork 10.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[GlobalISel] Add constant-folding of FP binops to combiner. #65230
Conversation
@arsenm this PR is ready for review |
ccdb60c
to
e18d3e0
Compare
e18d3e0
to
5bfc6a1
Compare
@@ -2645,6 +2645,14 @@ void CombinerHelper::replaceInstWithConstant(MachineInstr &MI, APInt C) { | |||
MI.eraseFromParent(); | |||
} | |||
|
|||
void CombinerHelper::replaceInstWithFConstant(MachineInstr &MI, ConstantFP *CFP) { | |||
assert(MI.getNumDefs() == 1 && "Expected only one def?"); | |||
APFloat C = CFP->getValueAPF(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't really need this temporary copy
void CombinerHelper::replaceInstWithFConstant(MachineInstr &MI, ConstantFP *CFP) { | ||
assert(MI.getNumDefs() == 1 && "Expected only one def?"); | ||
APFloat C = CFP->getValueAPF(); | ||
Builder.setInstr(MI); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should really just fix the combiner to always set the insert point before the apply
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed.
fad3cc7
to
34d91b6
Compare
I was waiting for you to approve before merging. |
For some reason GitHub makes it more difficult to find the approve without merge option |
No description provided.