[PATCH] Implement ISB memory barrier intrinsic for AArch32

Yi Kong kongy.dev at gmail.com
Thu Jul 3 07:33:12 PDT 2014


Thanks for reviewing! Patch updated.

Also moved regression tests to a new file as intrinsics-v8.ll is not
the right place to put them...

Thanks,
Yi

On 3 July 2014 14:47, Tim Northover <t.p.northover at gmail.com> wrote:
> Hi Yi,
>
> I think the Clang patch looks reasonable. The LLVM one has a couple of
> issues though:
>
>> def ISB : AInoP<(outs), (ins instsyncb_opt:$opt), MiscFrm, NoItinerary,
>>                 "isb", "\t$opt", []>,
>>                 Requires<[IsARM, HasDB]> {
>
> There's no ARM pattern here, which means that the Thumb instruction is
> actually being chosen during CodeGen (due to the next bug) and the
> object files produced will be incorrect.
>
>> def t2ISB : T2I<(outs), (ins instsyncb_opt:$opt), NoItinerary,
>> -                "isb", "\t$opt", []>, Requires<[HasDB]> {
>> +                "isb", "\t$opt", [(int_arm_isb (i32 imm0_15:$opt))]>,
>> +                Requires<[HasDB]> {
>
> This needs to be marked Requires<[IsThumb, HasDB]> (as do the
> surrounding instructions). I *think* it's undetectable at the moment
> (all paths I've tried happen to pick the ARM instruction first when
> available, purely by coincidence), so on this occasion limited tests
> are probably OK: I'd say all that's needed is MC-level tests that we
> can still assemble these on v6M cores. I can't see any, and we don't
> want that to break.
>
> +++ b/test/CodeGen/ARM/intrinsics-v8.ll
>
> Could you add a thumb RUN line here too?
>
> Cheers.
>
> Tim.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: llvm-ARM-Implement-memory-barriers-intrinsics.patch
Type: text/x-patch
Size: 7592 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140703/ea111f9e/attachment.bin>


More information about the llvm-commits mailing list