llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
26.71k stars 10.94k forks source link

Constant Hoist Makes Inconsistent Code Generation #40899

Open linzj opened 5 years ago

linzj commented 5 years ago
Bugzilla Link 41554
Version 7.0
OS Linux

Extended Description

include

struct MemChunk { int a, flags; }; void SlowFoo(void a, void b); static const int kPageSizeBits = 19; static inline MemChunk ToMemChunk(void o) { uintptr_t i = reinterpret_cast(o); const int page_mask = ~((1 << kPageSizeBits) - 1); i = i & page_mask; return reinterpret_cast<MemChunk>(i); } void foo(void a, void b) { MemChunk m_a = ToMemChunk(a); if ((m_a->flags & 2) == 0) return; MemChunk* m_b = ToMemChunk(b); if ((m_b->flags & 2) == 0) return; SlowFoo(a, b); }

compiles with -target armv7-linux-androideabi -fPIC -O3 -mllvm -debug -S -march=armv7-a -mfloat-abi=softfp -mfpu=neon -marm --sysroot=/home/linzj/android-ndk-r13b/platforms/android-15/arch-arm/ --gcc-toolchain=/home/linzj/android-ndk-r13b/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/ -fomit-frame-pointer -fvisibility=hidden

And gen

mov r2, #&#8203;4
mov r3, r0
bfi r3, r2, #&#8203;0, #&#8203;19
ldrb    r2, [r3]
tst r2, #&#8203;2
bxeq    lr
movw    r2, #&#8203;0
movt    r2, #&#8203;65528
and r2, r1, r2
ldrb    r2, [r2, #&#8203;4]
tst r2, #&#8203;2
bne .LBB0_2

@ %bb.1: bx lr .LBB0_2: b Z7SlowFooPvS

note that the first and is combined as bfi instruction, but the second one would not. I think mov r2, #​4 mov r3, r0 bfi r3, r2, #​0, #​19 ldrb r3, [r3] tst r3, #​2 bxeq lr mov r3, r1 bfi r3, r2, #​0, #​19 ldrb r2, [r3] tst r2, #​2 bne .LBB0_2 @ %bb.1: @ %cleanup7 bx lr .LBB0_2: @ %if.end6 b Z7SlowFooPvS

is better output.

linzj commented 5 years ago

SDValue SelectionDAGBuilder::getValue(const Value *V) { // If we already have an SDValue for this value, use it. It's important // to do this first, so that we don't create a CopyFromReg if we already // have a regular SDValue. SDValue &N = NodeMap[V]; if (N.getNode()) return N;

// Special handling BitCastInst with a constant. if (const BitCastInst I = dyn_cast(V)) { if (const Constant C = dyn_cast(I->getOperand(0))) { SDValue Val = getValueImpl(C); NodeMap[V] = Val; resolveDanglingDebugInfo(V, Val); return Val; } }

// If there's a virtual register allocated and initialized for this // value, use it. if (SDValue copyFromReg = getCopyFromRegs(V, V->getType())) return copyFromReg;

// Otherwise create a new SDValue and remember it. SDValue Val = getValueImpl(V); NodeMap[V] = Val; resolveDanglingDebugInfo(V, Val); return Val; }

seems okay for this case.