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

[Xinliang David Li via llvm-commits]

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
>>>>
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151105/6d4a9002/attachment.html>