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

[Xinliang David Li via llvm-commits]

On Thu, Nov 5, 2015 at 4:43 PM, David Majnemer <david.majnemer at gmail.com>
wrote:

> 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.
>

Technically it should be fine to use this type. However it will make the
field type different from what is declared in the coverage instrumentation
code.

David



>
> 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/dec63cd3/attachment-0001.html>