[cfe-commits] r54395 - in /cfe/trunk: lib/CodeGen/CGExpr.cpp test/CodeGen/2008-07-22-packed-bitfield-access.c

[Daniel Dunbar]

Yes, I actually looked at that while implementing the code. I assume
you meant padded instead of aligned.

Is this a case we really care about supporting? It is not immediately clear
to me that 
   struct __attribute((packed)) x {int a : 24;};
shouldn't have size sizeof(int) instead of size 3. There is a difference between 
the packing of the bitfield and the size of the structure. Is this a case which 
matters in practice?


 - Daniel


----- Original Message ----
From: Eli Friedman <eli.friedman at gmail.com>
To: Daniel Dunbar <daniel at zuster.org>
Cc: cfe-commits at cs.uiuc.edu
Sent: Saturday, August 9, 2008 4:21:52 PM
Subject: Re: [cfe-commits] r54395 - in /cfe/trunk: lib/CodeGen/CGExpr.cpp test/CodeGen/2008-07-22-packed-bitfield-access.c

On Tue, Aug 5, 2008 at 10:08 PM, Daniel Dunbar <daniel at zuster.org> wrote:
> +  // Fetch the high bits if necessary.
> +  if (LowBits < BitfieldSize) {
> +    unsigned HighBits = BitfieldSize - LowBits;
> +    llvm::Value *HighPtr =
> +      Builder.CreateGEP(Ptr, llvm::ConstantInt::get(llvm::Type::Int32Ty, 1),
> +                        "bf.ptr.hi");
> +    llvm::Value *HighVal = Builder.CreateLoad(HighPtr,
> +                                              LV.isVolatileQualified(),
> +                                              "tmp");

This is incorrect; it assumes that the struct is sufficiently aligned
for the load to succeed.  See
http://llvm.org/bugs/show_bug.cgi?id=2394 for a similar bug in
llvm-gcc.

-Eli