[PATCH] Implement ISB memory barrier intrinsic for AArch32

Tim Northover t.p.northover at gmail.com
Thu Jul 3 06:47:30 PDT 2014


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.



More information about the llvm-commits mailing list