-
Notifications
You must be signed in to change notification settings - Fork 13.3k
[llvm] Add support for llvm IR atomicrmw fminimum/fmaximum instructions #136759
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
fbb1c0e
f0ff4d6
b3ab5b7
9d97625
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -931,6 +931,8 @@ static Value *performMaskedAtomicOp(AtomicRMWInst::BinOp Op, | |
case AtomicRMWInst::FSub: | ||
case AtomicRMWInst::FMin: | ||
case AtomicRMWInst::FMax: | ||
case AtomicRMWInst::FMaximum: | ||
case AtomicRMWInst::FMinimum: | ||
Comment on lines
+934
to
+935
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing test coverage for this, should copy the fmin/fmax tests in test/Transforms/AtomicExpand. This is not the same as LowerAtomic which you did add the test for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, now added. Actually, fmin/fmax were missing, so I've added those to |
||
case AtomicRMWInst::UIncWrap: | ||
case AtomicRMWInst::UDecWrap: | ||
case AtomicRMWInst::USubCond: | ||
|
@@ -1819,6 +1821,8 @@ static ArrayRef<RTLIB::Libcall> GetRMWLibcall(AtomicRMWInst::BinOp Op) { | |
case AtomicRMWInst::UMin: | ||
case AtomicRMWInst::FMax: | ||
case AtomicRMWInst::FMin: | ||
case AtomicRMWInst::FMaximum: | ||
case AtomicRMWInst::FMinimum: | ||
case AtomicRMWInst::FAdd: | ||
case AtomicRMWInst::FSub: | ||
case AtomicRMWInst::UIncWrap: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8776,6 +8776,8 @@ Value *OpenMPIRBuilder::emitRMWOpAsInstruction(Value *Src1, Value *Src2, | |
case AtomicRMWInst::UMin: | ||
case AtomicRMWInst::FMax: | ||
case AtomicRMWInst::FMin: | ||
case AtomicRMWInst::FMaximum: | ||
case AtomicRMWInst::FMinimum: | ||
Comment on lines
+8779
to
+8780
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This code really should be deleted. I don't know why OMPIRBuilder has anything like this, but it's also a straight copy of the lower atomic utility There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks. I added it to avoid a compiler warning about unhandled case. |
||
case AtomicRMWInst::UIncWrap: | ||
case AtomicRMWInst::UDecWrap: | ||
case AtomicRMWInst::USubCond: | ||
|
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.
This needs to be added to the end of the enum to prevent a C ABI break.
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.
Thanks, now moved in latest patch.