[PATCH] D78663: [builtins] Add 32-bit shift builtins

Anatoly Trosinenko via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 13 03:04:58 PDT 2020


atrosinenko added a comment.

Thanks @aykevl! This could be useful for MSP430 as well. Especially, the tests would be worth reusing even for target-specific assembly re-implementation of these routines.

Another bit of "non functional" comments.



================
Comment at: compiler-rt/lib/builtins/int_ashl_impl.inc:14
+static inline ashl_int_t __ashlXi3(ashl_int_t a, int b) {
+  const int bits_in_word = (int)(sizeof(ashl_int_t) * CHAR_BIT) / 2;
+  ashl_words_t input;
----------------
As I understand, different widths were mentioned in `bits_in_?word` for different LibCalls (such as `bits_in_dword` for `__ashlti3`). Maybe something more generic such as `half_width_bits` would fit better.


================
Comment at: compiler-rt/lib/builtins/int_ashl_impl.inc:19
+  if (b & bits_in_word) {
+    // bits_in_word <= b < bits_in_sword
+    result.s.low = 0;
----------------
Then, this could be written as `half_width_bits <= b < `, hmm, `total_bits` - not actually declared as a constant, but hopefully would be useful for clarity.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78663



More information about the llvm-commits mailing list