Quuxplusone
commented
4 years ago Attached slub-ad4157.tar.xz (841312 bytes, application/x-xz): ```
slub-ad4157*
Closed Quuxplusone closed 4 years ago
Quuxplusone
commented
4 years ago Attached slub-ad4157.tar.xz (841312 bytes, application/x-xz): ```
slub-ad4157*
Jonas, unless the fix is immediately obvious from the attached reproducer,
please revert. We can then work on getting a more concise test case if the one
supplied is insufficient.
clang: ../lib/IR/InlineAsm.cpp:39:
llvm::InlineAsm::InlineAsm(llvm::FunctionType *, const std::string &, const
std::string &, bool, bool, llvm::InlineAsm::AsmDialect): Assertion
`Verify(getFunctionType(), constraints) && "Function type not legal for
constraints!"' failed.
...
3. ./arch/arm64/include/asm/atomic_lse.h:414:1
<Spelling=./arch/arm64/include/asm/atomic_lse.h:384:34>: Generating code for
declaration '__lse__cmpxchg_double'
...
#11 0x00000000036e95e0 llvm::InlineAsm::Verify(llvm::FunctionType*,
llvm::StringRef) (/android0/llvm-project/llvm/build/bin/clang-12+0x36e95e0)
#12 0x00000000036eb5fb llvm::InlineAsmKeyType::create(llvm::PointerType*) const
(/android0/llvm-project/llvm/build/bin/clang-12+0x36eb5fb)
#13 0x00000000036e9c73
llvm::ConstantUniqueMap<llvm::InlineAsm>::getOrCreate(llvm::PointerType*,
llvm::InlineAsmKeyType) (/android0/llvm-project/llvm/build/bin/clang-
12+0x36e9c73)
#14 0x00000000036e9954 llvm::InlineAsm::get(llvm::FunctionType*,
llvm::StringRef, llvm::StringRef, bool, bool, llvm::InlineAsm::AsmDialect)
(/android0/llvm-project/llvm/build/bin/clang-12+0x36e9954)
#15 0x000000000433b0fd
clang::CodeGen::CodeGenFunction::EmitAsmStmt(clang::AsmStmt const&)
(/android0/llvm-project/llvm/build/bin/clang-12+0x433b0fd)I'll need to preprocess the output, but the source definition looks like:
https://github.com/ClangBuiltLinux/linux/blob/b5fc7a89e58bcc059a3d5e4db79c481fb437de59/arch/arm64/include/asm/atomic_lse.h#L383-L415
399 asm volatile( \
400 __LSE_PREAMBLE \
401 " casp" #mb "\t%[old1], %[old2], %[new1], %[new2], %[v]\n"\
402 " eor %[old1], %[old1], %[oldval1]\n" \
403 " eor %[old2], %[old2], %[oldval2]\n" \
404 " orr %[old1], %[old1], %[old2]" \
405 : [old1] "+&r" (x0), [old2] "+&r" (x1), \
406 [v] "+Q" (*(unsigned long *)ptr) \
407 : [new1] "r" (x2), [new2] "r" (x3), [ptr] "r" (x4), \
408 [oldval1] "r" (oldval1), [oldval2] "r" (oldval2) \
409 : cl);(In reply to Nick Desaulniers from comment #2)
> Jonas, unless the fix is immediately obvious from the attached reproducer,
> please revert. We can then work on getting a more concise test case if the
> one supplied is insufficient.
>
> clang: ../lib/IR/InlineAsm.cpp:39:
> llvm::InlineAsm::InlineAsm(llvm::FunctionType *, const std::string &, const
> std::string &, bool, bool, llvm::InlineAsm::AsmDialect): Assertion
> `Verify(getFunctionType(), constraints) && "Function type not legal for
> constraints!"' failed.
> ...
> 3. ./arch/arm64/include/asm/atomic_lse.h:414:1
> <Spelling=./arch/arm64/include/asm/atomic_lse.h:384:34>: Generating code for
> declaration '__lse__cmpxchg_double'
> ...
> #11 0x00000000036e95e0 llvm::InlineAsm::Verify(llvm::FunctionType*,
> llvm::StringRef) (/android0/llvm-project/llvm/build/bin/clang-12+0x36e95e0)
> #12 0x00000000036eb5fb llvm::InlineAsmKeyType::create(llvm::PointerType*)
> const (/android0/llvm-project/llvm/build/bin/clang-12+0x36eb5fb)
> #13 0x00000000036e9c73
> llvm::ConstantUniqueMap<llvm::InlineAsm>::getOrCreate(llvm::PointerType*,
> llvm::InlineAsmKeyType)
> (/android0/llvm-project/llvm/build/bin/clang-12+0x36e9c73)
> #14 0x00000000036e9954 llvm::InlineAsm::get(llvm::FunctionType*,
> llvm::StringRef, llvm::StringRef, bool, bool, llvm::InlineAsm::AsmDialect)
> (/android0/llvm-project/llvm/build/bin/clang-12+0x36e9954)
> #15 0x000000000433b0fd
> clang::CodeGen::CodeGenFunction::EmitAsmStmt(clang::AsmStmt const&)
> (/android0/llvm-project/llvm/build/bin/clang-12+0x433b0fd)
I reverted the patch temporarily.
You seem to have encountered two problems: One segfault in MachineLICM, and one
assertion in clang. I only see a test case for the clang problem, but as you
say it is not preprocessed. Please provide reduced test cases for both problems
you see along with run-lines...I appreciate the quick revert. Beyond Sami's test, a lot of different trees of ours went red overnight for different ISAs: https://travis-ci.com/github/ClangBuiltLinux/continuous-integration/builds/189921001.
I'll need to detangle if they're all from the suspected change, or if there's more than one overnight breakage.
I should be able to provide reduced test cases today.
Quuxplusone
commented
4 years ago Reduced test case:
// clang -O2 --target=aarch64-linux-gnu slub.i -c
void __lse__cmpxchg_double_mb_ptr(void) {
register int a asm("x1");
asm("" : "+&r"(a));
}(In reply to Nick Desaulniers from comment #5)
> a lot of different trees
> of ours went red overnight for different ISAs:
> https://travis-ci.com/github/ClangBuiltLinux/continuous-integration/builds/
> 189921001.
>
> I'll need to detangle if they're all from the suspected change, or if
> there's more than one overnight breakage.
The 32b ARM, x86_64, and SystemZ/s390 failures also come from commit
c78da037783bda0f27f4d82060149166e6f0c796 ("[clang] Improve handling of physical
registers in inline assembly operands.")
I can provide reduced test cases for those ISAs if necessary, but I suspect the
test case for 64b ARM should be sufficient. I can also test all ISAs quickly
once the patch is updated.(In reply to Nick Desaulniers from comment #6)
> Reduced test case:
>
> // clang -O2 --target=aarch64-linux-gnu slub.i -c
>
> void __lse__cmpxchg_double_mb_ptr(void) {
> register int a asm("x1");
> asm("" : "+&r"(a));
> }
thanks for the test case - see Phabricator for discussion...42a8286
slub-ad4157.tar.xz(841312 bytes, application/x-xz)