LGTM, thanks!

LGTM.

In D126389#3549129, @arsenm wrote:

I wouldn’t really expect there to be a performance difference between gpr indexing and movrel requences. Gfx8 has both so you could directly compare there

Rebased, keep the current version for GFX9 and only apply this patch on architectures with movrel.

In D126389#3540486, @rampitec wrote:

In D126389#3540098, @jpages wrote:

In D126389#3539881, @foad wrote:

In D126389#3538346, @rampitec wrote:

Any performance numbers? The 8 element case was driven by a specific customer program and the performance of the cmp/select was better than movrel.

I don't know why that would be. Maybe the performance characteristics are different on GFX10+ compared to GFX9.

Also on GFX10+ sgpr usage does not affect occupancy, so perhaps the heuristic could be tweaked to make it more likely to use s_movrel (not v_movrel) on GFX10+.

I will try to get some performance numbers on specific games. Do you know if this performance problem was specific to an architecture?
Like Jay said, I could tweak the heuristic for this and only generate it for GFX10+.

All the tests were done on gfx9 flavors. It is also worth noting gfx9 does not have movrel, but uses s_set_gpr_idx_on instead, which may be the difference.

I.e. it may be linked to FeatureMovrel.

In D126389#3539881, @foad wrote:

In D126389#3538346, @rampitec wrote:

Any performance numbers? The 8 element case was driven by a specific customer program and the performance of the cmp/select was better than movrel.

I don't know why that would be. Maybe the performance characteristics are different on GFX10+ compared to GFX9.

Also on GFX10+ sgpr usage does not affect occupancy, so perhaps the heuristic could be tweaked to make it more likely to use s_movrel (not v_movrel) on GFX10+.

Updated a testcase

Rebased following Jay's comments.

Updated a failing test on AArch64.

Rebased following Jay's comments.

Rebased, as suggested I added a target independent ISD opcode.

The intrinsic has the IntrNoMem attribute, and is represented by INTRINSIC_WO_CHAIN in the DAG.

I changed the patch to have only one intrinsic with a rounding mode parameter as suggested. This required many modifications in the frontend parts of LLVM.

Added G_FPTRUNC_ROUND_UPWARD/G_FPTRUNC_ROUND_DOWNWARD opcodes between the intrinsics and the pseudo instructions. Added a custom ISD node for the DAG version.

In D110579#3054424, @foad wrote:

In D110579#3051790, @jpages wrote:

The consequence of this change is the use of setreg instead of s_round_mode in the codegen. But it's probably better to not reinvent the wheel as this pass is already optimized to not insert too many setreg.

Good point. s_round_mode/s_denorm_mode are new in GFX10, so they did not exist when this pass was written. Do you think the pass could be improved to emit s_round_mode/s_denorm_mode instead of s_setreg whenever it only needs to change the rounding/denormal bits of the mode register? That could be a separate patch.

It was possible without the script, the diff is smaller like that, it's probably better.

Rebased: updated the test with the script instead of manually.

Rebased without -LABEL in --check-prefixes

Rebased based on Jay's comments.

Rebased based on previous comments.

In D103344#2793853, @arsenm wrote:

Probably should also add a globalisel test just in case

Renamed the test file with a better name.

Rebased, changed the test with bugpoint/opt to a much simpler version.

Added the VOP3Mods in the pattern and associated tests, thanks for the suggestion!

Rebased to it in Tablegen, it should be cleaner now. I removed the too complex pattern with the conversions from i16 to f16.
Instead this v_pack_b32_f16 instruction will be used only for f16 when we are sure the two inputs have already been flushed if needed.

Thanks for the review. I don't have commit access to the repo, could someone do it for me?

Updated for code style

Thank you for your inputs.

It appears that this instruction is harder to select than expected on integers.