Details
- Reviewers
reames javed.absar asb jyknight
Diff Detail
Event Timeline
lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp | ||
---|---|---|
2171 | We don't support any other cases where a narrower FP type is illegal, but a wider one is. That would require a generalized version of FP16_TO_FP/FP_TO_FP16. There aren't any in tree targets that support f64 but not f32 for example. I've removed this assertion, so this will fail in GetPromotionOpcode for any other type like any other legalization would |
Fix handling with wider illegal FP types. Also avoid directly using the f16 legalization instructions
f80 is illegal since it's not a power of two sized type, so that's rejected by the IR verifier. I guess supporting it could somehow be possible, but is beyond the scope of this patch. I added some tests for fp128.
Ideally each target would add support for the FP types directly in its selection patterns, but that's a lot of work. The individual targets avoid work by setting the defaults to the bitcast lowering. For the atomic_load/store FP support, this for some reason was done in the IR AtomicExpand pass.
I should probably plan to reflect support for this in my in-flight cmpxchg patch (which adds target-independent support for late lowering of cmpxchg in the same way I added it for atomicrmw). [incidentally - reviews on that patch would be very welcome...]
Adding James Y Knight as a reviewer here too.
EDIT: worth actually linking to my cmpxchg patch! D48131
Looks reasonable.
Perhaps add support for pointer types too? Supporting the same set of types for load, store, xchg, and cmpxchg (eventually) would seem sensible.
lib/CodeGen/AtomicExpandPass.cpp | ||
---|---|---|
504 | Add a comment here that this code can go away if the cmpxchg instruction adds support for floating point types. |
lib/CodeGen/AtomicExpandPass.cpp | ||
---|---|---|
504 | I think there might need to be a separate fcmpxchg instruction for that, unless you mean there will also be a version that treats the FP type here as integer in memory |
lib/CodeGen/AtomicExpandPass.cpp | ||
---|---|---|
504 | No -- the intent is not to compare for floating-point-equality ala fcmp, but rather just as bit equality. (e.g. NaNs are equal to each-other when they have the same bit representation, and unequal if they do not) |
lib/CodeGen/AtomicExpandPass.cpp | ||
---|---|---|
504 | This is what various ISAs and programming languages do, so I support what James says :) |
lib/CodeGen/AtomicExpandPass.cpp | ||
---|---|---|
504 | AMDGPU does also have fcmpxchg, which does an FP compare, with nans failing etc. |
lib/CodeGen/AtomicExpandPass.cpp | ||
---|---|---|
504 | Off topic... but wat?!? Once you have a NaN your fcmpxchg infloops? Count me confused. |
xxz
lib/CodeGen/AtomicExpandPass.cpp | ||
---|---|---|
504 | It doesn't let you put the NaN in? As long as the memory was initialized with something not-NaN before I think it works? Otherwise you're stuck with NaNs forever. I haven't tried using it, but that's my interpretation of the manual. |
lib/CodeGen/AtomicExpandPass.cpp | ||
---|---|---|
504 | atomicrmw xchg with FP types makes sense - the semantics are unambiguous. Is it really worth the potential confusion of what fp cmpxchg means vs just sticking with bitcast + integer cmpxchg? |
lib/CodeGen/AtomicExpandPass.cpp | ||
---|---|---|
504 | To be clear I think fcmpxchg is it's own operation separate from the current cmpxchg inst |
lib/CodeGen/AtomicExpandPass.cpp | ||
---|---|---|
504 | I hope we never support "fcmpxchg" -- that is, using a floating point semantic comparison. I'm having trouble imagining when that could ever be a useful operation. Whether or not we support bitwise cmpxchg with FP types I'm pretty agnostic to. If there's some reason why it's useful to do so, we should. If there isn't, maybe we should or maybe we shouldn't. But -- to the point of this thread: there's now a comment here that this code should be removed if we do so, which is all I really wanted. :) |
Add a comment here that this code can go away if the cmpxchg instruction adds support for floating point types.