Add an intrinsic that takes 2 integers and perform saturation addition on them.
This is a part of implementing fixed point arithmetic in clang where some of the more complex operations will be implemented as intrinsics.
Paths
| Differential D53053
[Intrinsic] Signed Saturation Addition Intrinsic ClosedPublic Authored by leonardchan on Oct 9 2018, 4:30 PM.
Details Summary Add an intrinsic that takes 2 integers and perform saturation addition on them. This is a part of implementing fixed point arithmetic in clang where some of the more complex operations will be implemented as intrinsics.
Diff Detail
Event Timeline
craig.topper retitled this revision from [Intrinsic] Signed Saturation Addition Intirnsic to [Intrinsic] Signed Saturation Addition Intrinsic.Oct 9 2018, 9:40 PM
leonardchan marked 9 inline comments as done. leonardchan added inline comments.
leonardchan marked 6 inline comments as done. leonardchan added inline comments.
Comment Actions Looks good to me! Though, I just realized that I think you need to add the new node to TargetSelectionDAG.td. Unsure if it needs to be added somewhere else as well.
Comment Actions Are we not supporting vectors at all? That needs to be checked in the IR verifier if so. Though I would really like to see this for vectors since X86 has 8 and 16 bit saturating vector addition.
leonardchan marked 4 inline comments as done. Comment Actions
We can add them. Not sure if what I added is the correct way they should be expanded though. I might just not be looking hard enough, but I couldn't find other examples of intrinsics that take int vector arguments and expand them.
Comment Actions Intrinsics with vector operands are expanded in LegalizeVectorOps.cpp. Sometimes with a vectorized sequence or in the worst case by unrolling. Comment Actions
Added the appropriate case in LegalizeVectorOps. This should eventually allow the node to be unrolled in Expand, which I think does the same thing I had before in a previous diff of this patch.
leonardchan marked 5 inline comments as done. leonardchan added inline comments.
This revision is now accepted and ready to land.Oct 15 2018, 9:56 PM Closed by commit rL344629: [Intrinsic] Signed Saturation Addition Intrinsic (authored by leonardchan). · Explain WhyOct 16 2018, 10:37 AM This revision was automatically updated to reflect the committed changes. Comment Actions
Was going to work on that within the next few days. Right now I'm working on finishing up for a clang patch that uses these add intrinsics for fixed point addition. Comment Actions I don't see this or its unsigned counterpart added to the LangRef; is that intentional, or an oversight? Comment Actions @JosephTremoulet Made a patch for the documentation (https://reviews.llvm.org/D54729) spatel mentioned this in D96904: [IR] restrict vector reduction intrinsic types.Feb 19 2021, 9:23 AM
Revision Contents
Diff 169720 include/llvm/CodeGen/ISDOpcodes.h
include/llvm/CodeGen/TargetLowering.h
include/llvm/IR/Intrinsics.td
include/llvm/Target/TargetSelectionDAG.td
lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
lib/CodeGen/SelectionDAG/LegalizeTypes.h
lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp
lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
lib/CodeGen/SelectionDAG/SelectionDAGDumper.cpp
lib/CodeGen/SelectionDAG/TargetLowering.cpp
lib/CodeGen/TargetLoweringBase.cpp
lib/IR/Verifier.cpp
test/CodeGen/X86/sadd_sat.ll
|
This is a nitpick but I think it would be better if it were named SADDSAT to match the naming of the other nodes (SADDO, SMULO for example).