[llvm] r252099 - Define portable macros for packed struct definitions:

Thu Nov 5 16:43:27 PST 2015

Is there any reason why you did want to use unaligned_int16_t, etc from
<llvm/Support/Endian.h>?  Having fewer ways of doing things is, in general,
nicer.

On Thu, Nov 5, 2015 at 9:11 AM, Xinliang David Li via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

>
>
> On Thu, Nov 5, 2015 at 12:02 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
>>
>>
>> On Wed, Nov 4, 2015 at 9:36 PM, Xinliang David Li <davidxl at google.com>
>> wrote:
>>
>>>
>>>
>>> On Wed, Nov 4, 2015 at 8:18 PM, David Blaikie <dblaikie at gmail.com>
>>> wrote:
>>>
>>>>
>>>>
>>>> On Wed, Nov 4, 2015 at 3:42 PM, Xinliang David Li via llvm-commits <
>>>> llvm-commits at lists.llvm.org> wrote:
>>>>
>>>>> Author: davidxl
>>>>> Date: Wed Nov  4 17:42:56 2015
>>>>> New Revision: 252099
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=252099&view=rev
>>>>> Log:
>>>>> Define portable macros for packed struct definitions:
>>>>>
>>>>>  1. A macro with argument: LLVM_PACKED(StructDefinition)
>>>>>  2. A pair of macros defining scope of region with packing:
>>>>>     LLVM_PACKED_START
>>>>>      struct A { ... };
>>>>>      struct B { ... };
>>>>>     LLVM_PACKED_END
>>>>>
>>>>
>>>> What are we planning to use these for (I imagine there are already some
>>>> uses of packed macros/etc in various parts of LLVM that this will be used
>>>> to replace? What are they? What new uses do you have in mind?) I'm a little
>>>> concerned, because this tend to leads to non-portable code trying to parse
>>>> & then use structs with misaligned members, etc... (which, if I understand
>>>> correctly, is only a perf penalty on X86, but potentially a correctness
>>>> problem on other platforms we support/care about (not to mention the known
>>>> bits stuff we do might assume certain bits are zero when they aren't, etc))
>>>>
>>>>
>>>
>>> This is used for existing runtime data structure (Coverage Map's
>>> Function Record). Previously the structure is not explicitly declared but
>>> referenced with manually specified layout and alignment.
>>>
>>
>> Not sure I quite follow this ^ sorry - could you give an example?
>> (perhaps this commit could've included an example of porting over an
>> existing use to use the new macros from whatever the code was doing
>> previously)
>>
>>
>
> See http://reviews.llvm.org/D13843 and see my replies there.
>
>
>
>> New uses of the macro should be done with care -- but there might be
>>> legit reason to use it though, let's not rule that out.
>>>
>>
>> Might be, though ideally I'd prefer to see at least a motivating use case
>> before adding tools like this, but I'm not strongly invested/pushing back
>> here.
>>
>
> I have plans to restructure the profile data structures. If I manage to
> get rid of the support of the legacy format, we can revisit this topic
> (about the pros and cons etc).
>
> David
>
>
>>
>>
>>>
>>> David
>>>
>>>
>>>
>>>>
>>>>> Differential Revision: http://reviews.llvm.org/D14337
>>>>>
>>>>> Modified:
>>>>>     llvm/trunk/include/llvm/Support/Compiler.h
>>>>>
>>>>> Modified: llvm/trunk/include/llvm/Support/Compiler.h
>>>>> URL:
>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/Compiler.h?rev=252099&r1=252098&r2=252099&view=diff
>>>>>
>>>>> ==============================================================================
>>>>> --- llvm/trunk/include/llvm/Support/Compiler.h (original)
>>>>> +++ llvm/trunk/include/llvm/Support/Compiler.h Wed Nov  4 17:42:56 2015
>>>>> @@ -293,6 +293,34 @@
>>>>>  # define LLVM_ALIGNAS(x) alignas(x)
>>>>>  #endif
>>>>>
>>>>> +/// \macro LLVM_PACKED
>>>>> +/// \brief Used to specify a packed structure.
>>>>> +/// LLVM_PACKED(
>>>>> +///    struct A {
>>>>> +///      int i;
>>>>> +///      int j;
>>>>> +///      int k;
>>>>> +///      long long l;
>>>>> +///   });
>>>>> +///
>>>>> +/// LLVM_PACKED_START
>>>>> +/// struct B {
>>>>> +///   int i;
>>>>> +///   int j;
>>>>> +///   int k;
>>>>> +///   long long l;
>>>>> +/// };
>>>>> +/// LLVM_PACKED_END
>>>>> +#ifdef _MSC_VER
>>>>> +# define LLVM_PACKED(d) __pragma(pack(push, 1)) d __pragma(pack(pop))
>>>>> +# define LLVM_PACKED_START __pragma(pack(push, 1))
>>>>> +# define LLVM_PACKED_END   __pragma(pack(pop))
>>>>> +#else
>>>>> +# define LLVM_PACKED(d) d __attribute__((packed))
>>>>> +# define LLVM_PACKED_START _Pragma("pack(push, 1)")
>>>>> +# define LLVM_PACKED_END   _Pragma("pack(pop)")
>>>>> +#endif
>>>>> +
>>>>>  /// \macro LLVM_PTR_SIZE
>>>>>  /// \brief A constant integer equivalent to the value of
>>>>> sizeof(void*).
>>>>>  /// Generally used in combination with LLVM_ALIGNAS or when doing
>>>>> computation in
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>>
>>>>
>>>>
>>>
>>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151105/cf14e54e/attachment.html>