[PATCH] D93101: [Clang][Codegen] Truncate initializers of union bitfield members

[Tomas Matheson via Phabricator via cfe-commits]

tmatheson marked 3 inline comments as done.
tmatheson added inline comments.


================
Comment at: clang/lib/AST/ExprConstant.cpp:9801-9804
+    return EvaluateInPlace(Result.getUnionValue(), Info, Subobject, InitExpr) ||
+           (Field->isBitField() &&
+            truncateBitfieldValue(Info, InitExpr, Result.getUnionValue(),
+                                  Field));
----------------
rsmith wrote:
> Shouldn't this be `&&` not `||`? These functions return `true` if they succeed (unlike the convention in `Sema` where `true` means an error diagnostic was produced).
You are correct. I have updated the logic to be simpler to follow as well.


================
Comment at: clang/test/CodeGenCXX/bitfield-layout.cpp:88-95
+// CHECK: define i32 @_Z10test_truncv()
+int test_trunc() {
+  union {
+    int i : 4;
+  } U = {15};
+  return U.i;
+  // CHECK: ret i32 -1
----------------
joechrisellis wrote:
> I'd like to see some more tests that check the truncation behaviour. My understanding is that this is trucating to -1 because of two's complement? How about something like:
> 
> ```
> int test_trunc() {
>     union {
>         int i : 4;
>     } U = {80};
>     return U.i;
>     // CHECK: ret i32 0
> }
> ```
> 
> Am I understanding the behaviour correctly?
> 
> Some comments about what is actually happening on the bit-level to get this result would also be nice.
Yes that is correct. I have added some new test cases and comments to make it clear what they are testing for.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93101