Hi Tim, Thanks for the explanation. The above code snippets is actually extracted by bugpoint from MultiSource/Applications/ALAC/encode/alacconvert-encode.test. I get a different result if I cast the pointer to int and mask away the lower two bits and cast the int back to pointer. While casting pointer to int and back to pointer do not cause any problem. Thanks Hongbin On Fri, Dec 16, 2016 at 12:14 PM, Tim Northover <t.p.northover at gmail.com> wrote:> Hi Hongbin, > > On 16 December 2016 at 11:57, Hongbin Zheng via llvm-dev > <llvm-dev at lists.llvm.org> wrote: > > We have an "align 4" in the StoreInst. Does this mean > > 1) the address 'A' should be aligned to 4 bytes? > > 2) the lower 2 bits of 'A' should be always 0? > > Yes, these are both the same. It means the compiler will assume those > facts when reasoning about what that store might do and possibly when > choosing what instruction to use to implement it. > > Chers. > > Tim. >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20161216/9a6e71af/attachment.html>
Hi Hongbin, On 16 December 2016 at 12:22, Hongbin Zheng <etherzhhb at gmail.com> wrote:> Thanks for the explanation. The above code snippets is actually extracted by > bugpoint from MultiSource/Applications/ALAC/encode/alacconvert-encode.test. > I get a different result if I cast the pointer to int and mask away the > lower two bits and cast the int back to pointer. While casting pointer to > int and back to pointer do not cause any problem.It's quite possible there's some undefined behaviour in ALAC that's only recently become a problem. On almost all targets I'm aware of 32-bit integers should be aligned to 32-bits. There's a bit more variation for 64-bit ints. Programs that work on buffers of bytes often cast pointers into those buffers inappropriately. E.g. char *Buf = ...; [...] return doSomethingWithInts((int32_t *)&Buf[Idx]); For example I'd be very suspicious of ALACEncoder.cpp lines 340 and 352. I expect there's more, those are just the first two I found by grepping for "(int32_t *)". Everything seems to work fine until the compiler becomes clever enough to realise it can use a slightly faster store since the pointer is aligned (or one of any number of other optimizations), and then things blow up. Generally you need to memcpy part of the buffer into a properly aligned variable and pass that, or use a carefully under-aligned type. In Clang you can create a specifically under-aligned integer type, and as long as all *accesses* go through that type it should work. So something like this: typedef int32_t __attribute__((aligned(1))) unaligned_int32_t; [...] int32_t doSomethingWithInts(unaligned_int32_t *Ints) { return Ints[0] + Ints[1]; } But that's obviously a non-standard extension (GCC supports it, and I believe MSVC has a similar but incompatible concept). It's also really easy to mess that up, the attribute gets dropped in some really surprising situations you (or at least I) wouldn't expect. Cheers. Tim.
Thanks again. So this is actually a problem in the test case, and I can assume that I can trust alignment of the storeinst :) Let me try to modify that testcase. Thanks Hongbin On Fri, Dec 16, 2016 at 12:48 PM, Tim Northover <t.p.northover at gmail.com> wrote:> Hi Hongbin, > > On 16 December 2016 at 12:22, Hongbin Zheng <etherzhhb at gmail.com> wrote: > > Thanks for the explanation. The above code snippets is actually > extracted by > > bugpoint from MultiSource/Applications/ALAC/encode/alacconvert-encode. > test. > > I get a different result if I cast the pointer to int and mask away the > > lower two bits and cast the int back to pointer. While casting pointer to > > int and back to pointer do not cause any problem. > > It's quite possible there's some undefined behaviour in ALAC that's > only recently become a problem. On almost all targets I'm aware of > 32-bit integers should be aligned to 32-bits. There's a bit more > variation for 64-bit ints. > > Programs that work on buffers of bytes often cast pointers into those > buffers inappropriately. E.g. > > char *Buf = ...; > [...] > return doSomethingWithInts((int32_t *)&Buf[Idx]); > > For example I'd be very suspicious of ALACEncoder.cpp lines 340 and > 352. I expect there's more, those are just the first two I found by > grepping for "(int32_t *)". > > Everything seems to work fine until the compiler becomes clever enough > to realise it can use a slightly faster store since the pointer is > aligned (or one of any number of other optimizations), and then things > blow up. > > Generally you need to memcpy part of the buffer into a properly > aligned variable and pass that, or use a carefully under-aligned type. > In Clang you can create a specifically under-aligned integer type, and > as long as all *accesses* go through that type it should work. So > something like this: > > typedef int32_t __attribute__((aligned(1))) unaligned_int32_t; > [...] > int32_t doSomethingWithInts(unaligned_int32_t *Ints) { > return Ints[0] + Ints[1]; > } > > But that's obviously a non-standard extension (GCC supports it, and I > believe MSVC has a similar but incompatible concept). It's also really > easy to mess that up, the attribute gets dropped in some really > surprising situations you (or at least I) wouldn't expect. > > Cheers. > > Tim. >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20161216/9dde1c75/attachment.html>