[PATCH] Fix arm sbfx/ubfx generation

Hi,

Currently llvm can generate an invalid sbfx/ubfx op, for instance when compiling encoder/analyse.c in x264 (I'll reduce this to a test case if needed):

clang -O3 -Wall -I. -isysroot=/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS3.0.sdk -Qunused-arguments -mno-thumb -mdynamic-no-pic -arch armv7 -mcpu=cortex-a8 -mfpu=neon -mfloat-abi=softfp -DHAVE_ARMV6 -DHAVE_ARMV6T2 -DHAVE_NEON -DARCH_ARM -DSYS_MACOSX -DHAVE_PTHREAD -s -fomit-frame-pointer -I/usr/X11/include -c -o encoder/analyse.o encoder/analyse.c
/var/folders/WC/WCQfew6LEaqrFEF+j4W8CE+++TI/-Tmp-/cc-4koben.s:10000:immediate value out of range -- `sbfx r12,r12,#-2,#31'
/var/folders/WC/WCQfew6LEaqrFEF+j4W8CE+++TI/-Tmp-/cc-4koben.s:10041:immediate value out of range -- `sbfx r12,r0,#-2,#31'
/var/folders/WC/WCQfew6LEaqrFEF+j4W8CE+++TI/-Tmp-/cc-4koben.s:10336:immediate value out of range -- `sbfx r2,r8,#-2,#31'
/var/folders/WC/WCQfew6LEaqrFEF+j4W8CE+++TI/-Tmp-/cc-4koben.s:10362:immediate value out of range -- `sbfx r2,r6,#-2,#31'
/var/folders/WC/WCQfew6LEaqrFEF+j4W8CE+++TI/-Tmp-/cc-4koben.s:13229:immediate value out of range -- `sbfx r0,r0,#-2,#31'
/var/folders/WC/WCQfew6LEaqrFEF+j4W8CE+++TI/-Tmp-/cc-4koben.s:13235:immediate value out of range -- `sbfx r0,r3,#-2,#31'
clang: error: assembler command failed with exit code 1 (use -v to see invocation)
make: *** [encoder/analyse.o] Error 1

LSB isn't checked to ensure it's non-negative. In addition, LSB + Width cannot be >32, since it reduces to (Srl_imm - Shl_imm) + (32 - Srl_imm) == 32 - Shl_imm, and it was already assert()ed that 0 < Shl_imm < 32. So that check is just replaced by if (LSB < 0)

llvm-arm-neg-bfx.diff (577 Bytes)

Hi,

Currently llvm can generate an invalid sbfx/ubfx op, for instance when compiling encoder/analyse.c in x264 (I'll reduce this to a test case if needed):

Well I just read the dev policy and saw that this was required, so test case attached. It'll break if llvm starts emitting the lsl r0, r0, #3 / asr r0, r0, #1 variants for shifts, not sure how to handle that.

llvm-arm-neg-bfx-2.diff (1 KB)

Thanks. I've applied your patch after tweaking the test.

Evan