Building an s390 linux allmodconfig kernel produced an error message for a misoptimized code path: pblk-rb.i:...: error: invalid operand for inline asm constraint 'i' I managed to reduce the test case and make it architecture independent, test with "clang-9 pblk-rb.i -Wall -O2 -S": typedef struct { long counter; } atomic64_t; static inline void __atomic64_add_const(long val, long *ptr) { asm volatile (" agsi %[ptr],%[val]" :[ptr] "+Q"(*ptr) :[val] "i"(val) :"cc", "memory"); } void __atomic64_add_var(long val, long *ptr); static inline void atomic64_add(long i, atomic64_t * v) { if (__builtin_constant_p(i) && (i > -129) && (i < 128)) __atomic64_add_const(i, &v->counter); else __atomic64_add_var(i, &v->counter); } void pblk_rb_read_to_bio(atomic64_t *pblk, int count) { if (count) atomic64_add(count, pblk); atomic64_add(count, pblk); } The __builtin_constant_p() should never hit here, since 'count' is clearly not constant. If a special case is made for 'count=0', then I would expect 'val' to be 0 in that branch and fit into an "i" constraint.
Looks like we're probably missing a DCE step after lowering @llvm.is.constant calls to i1 false; we emit this in the assembly: movb $1, %al testb %al, %al jne .LBB0_6
This misbehavior of __builtin_constant_p() is blocking getting the kernel's FORTIFY_SOURCE working with Clang. extern unsigned char __linker_section_start[]; extern unsigned char __linker_section_end[]; extern void expected(void); extern void should_be_optimized_away(void); void foo(void) { if (__builtin_constant_p(__linker_section_end - __linker_section_start)) { /* Clang thinks it knows this size?! */ should_be_optimized_away(); } else { /* GCC correctly thinks it does not know this size. */ expected(); } } (Oddly, if the __linker_section* variables are "char *", this works as expected. However, converting all "char []" linker sections in the kernel to "char *" isn't going to be welcome.)
The frontend emits: %1 = call i1 @llvm.is.constant.i64(i64 sub (i64 ptrtoint ([0 x i8]* @__linker_section_end to i64), i64 ptrtoint ([0 x i8]* @__linker_section_start to i64))), !dbg !13 ... which should get optimized to i1 false. Per the LangRef: > This intrinsic generates no code. If its argument is known to be a manifest compile-time constant value, then the intrinsic will be converted to a constant true value. Otherwise, it will be converted to a constant false value. > > In particular, note that if the argument is a constant expression which refers to a global (the address of which _is_ a constant, but not manifest during the compile), then the intrinsic evaluates to false. It seems to me that the difference between the addresses of two globals, which might be a (static or dynamic) link-time constant, does not qualify here. This looks like an LLVM lowering bug to me.
It looks like LLVM considers: @real_mode_blob_end = external dso_local global [0 x i8], align 1 define i1 @bar() { %1 = call i1 @llvm.is.constant.i64(i64 ptrtoint ([0 x i8]* @real_mode_blob_end to i64)) ret i1 %1 } as a Constant: define i1 @bar() local_unnamed_addr #1 { ret i1 true } All lowerIsConstantIntrinsic in llvm/lib/Transforms/Scalar/LowerConstantIntrinsics.cpp does is return true if the lone operand to the intrinsic isa<Constant>. So should [0x<anything>]* not be considered Constant?
That behavior seems both to directly violate the LangRef: "In particular, note that if the argument is a constant expression which refers to a global (the address of which _is_ a constant, but not manifest during the compile), then the intrinsic evaluates to false." ... and to be incompatible with what we need for __builtin_constant_p lowering (which IIRC was the reason the intrinsic was added). I think we should only return true for: -- ConstantData other than UndefValue -- ConstantAggregate whose elements recursively are one of these (though for __builtin_constant_p lowering, all we care about is the case where the argument is an integer, for which we want to require that the argument is a ConstantInt)
https://reviews.llvm.org/D102367
https://reviews.llvm.org/rG8c72749bd92d35397e93908bc5a504d4cbcef1cb fixes Kees' report. Tom, may I cherry-pick that to release/12.x branch so that we can get fixes for the Linux kernel's FORTIFY_SOURCE string routines into users' hands faster than the clang-13 release??
(In reply to Nick Desaulniers from comment #7) > https://reviews.llvm.org/rG8c72749bd92d35397e93908bc5a504d4cbcef1cb > > fixes Kees' report. Tom, may I cherry-pick that to release/12.x branch so > that we can get fixes for the Linux kernel's FORTIFY_SOURCE string routines > into users' hands faster than the clang-13 release?? Yes, remember to use git cherry-pick -x.
Pushed to release/12.x as de579bae6eabd02b815e549776b8c680957a0769. Original bug report is also no longer reproducible; closing. Please re-open if I'm mistaken.