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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

Shall we factor out the common part of lshrti3.c and lshrdi3.c and place that in an `.inc` file? We can thus avoid duplication.



================
Comment at: compiler-rt/lib/builtins/lshrsi3.c:24
+  input.all = a;
+  if (b & bits_in_word) /* bits_in_word <= b < bits_in_sword */ {
+    result.s.high = 0;
----------------
Prefer BCPL-style `//` comments to C-style `/* */`

```lang=cpp
if (...) {
  // Comment
  ...
} else {
  // Comment
  ...
}
```


================
Comment at: compiler-rt/test/builtins/Unit/lshrsi3_test.c:24
+
+int test__lshrsi3(si_int a, int b, si_int expected)
+{
----------------
Place `{` in the end of the previous line.


================
Comment at: compiler-rt/test/builtins/Unit/lshrsi3_test.c:26
+{
+    si_int x = __lshrsi3(a, b);
+    if (x != expected)
----------------
The LLVM coding standard uses indent=2. I know that certain compiler-rt are using 4, but for new files, we probably should stick with indent=2.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78663