[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