[PATCH] D96501: [FPEnv][ARM] Implement lowering of llvm.set.rounding

[Simon Tatham via Phabricator via llvm-commits]

simon_tatham added a comment.

The table-lookup strategy seems like overkill to me. As far as I can see, the integer mapping required is: 0→3, 3→2. 2→1, 1→0. In other words, all four input values (that can be handled at all) are just reduced by 1, mod 4. So instead of `(147 >> (value << 1)) & 3`, you could compute `(value - 1) & 3`, surely more cheaply.

What about `RoundingMode::NearestTiesToAway`? There's no support for that in the FPSCR dynamic rounding-mode field, but it looks as if this patch will lower `@llvm.set.rounding(i32 4)` to //some// kind of code that silently does the wrong thing.

Perhaps that should have been error-checked by whatever generated the `@llvm.set.rounding` call in the first place? If so, I think at least a mention in the comments would be useful.



================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:6131
+  // a table, which consists of a sequence of 2-bit fields, each representing
+  // corresponding RISCV mode.
+  static const unsigned Table =
----------------
Corresponding Arm mode, surely!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96501/new/

https://reviews.llvm.org/D96501