Lower umin, umax, smin, smax intrinsics to corresponding UMIN, UMAX, SMIN, SMAX
instructions when feat CSSC is available.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
I have tried to collapse the to check test output for the different run configuration as we discussed on the SelDag ticket for this (D138813). However I could not wrangle the prefixes in a configuration that would make the update script output sensible checks that would cover all 4 cases. Hand-crafting is not a good route I think, especially since even though spelling out all 4 outputs is a bit verbose, it doesn't make things less clear, which should be the more important measure I think.
llvm/include/llvm/CodeGen/GlobalISel/LegalizerInfo.h | ||
---|---|---|
954 ↗ | (On Diff #480446) | Can you add a test for this function in llvm/unittests/CodeGen/GlobalISel/LegalizerInfoTest.cpp? Then maybe split this into two patches: Patch 1: Add minScalarIf |
llvm/include/llvm/CodeGen/GlobalISel/LegalizerInfo.h | ||
---|---|---|
954 ↗ | (On Diff #480446) | thanks, I made a separate patch for minScalarIf. |
llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp | ||
---|---|---|
197–202 | Should prefer to check all legal cases first |
llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp | ||
---|---|---|
194–202 | You can also conditionally add the predicates based on the subtarget feature check. i.e. you don't need to check the predicates inside the lambda, you can append rules for different subtargets. |
llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp | ||
---|---|---|
194–202 | Right. Indeed, I didn't think of that. Reviewing I still prefer the lambda-inlining. Maybe because I'm giddy to finally have an excuse to write lots of lambdas in C++. From your phrasing I think you're also ok with the current setup. |
llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp | ||
---|---|---|
194–202 | These are called a lot |
llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp | ||
---|---|---|
194–202 | If you're happy with this, I'll do the same to the other outstanding patches, lambda's be damned. |
llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp | ||
---|---|---|
194–202 | I think Matt's right in general, although IMO if it becomes extremely unwieldy for some reason we can embed the checks into the predicates (its ultimately not a big deal with these uncommon operations). |
clang-format not found in user’s local PATH; not linting file.