Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

selected processor does not support `bfc w0,#1,#5' #697

Closed
tpgxyz opened this issue Sep 24, 2019 · 11 comments
Closed

selected processor does not support `bfc w0,#1,#5' #697

tpgxyz opened this issue Sep 24, 2019 · 11 comments
Assignees
Labels
[ARCH] arm64 This bug impacts ARCH=arm64 [BUG] llvm A bug that should be fixed in upstream LLVM [FIXED][LLVM] 10 This bug was fixed in LLVM 10.0

Comments

@tpgxyz
Copy link

tpgxyz commented Sep 24, 2019

Trying to compile kernel 5.3.1 with LLVM/clang-9.0.0 on aarch64

Full logs can be found here https://abf.openmandriva.org/build_lists/610527 also links to GitHub sources

CC [M] drivers/gpu/drm/amd/amdgpu/sdma_v4_0.o BUILDSTDERR: /tmp/sdma_v4_0-3f5dad.s: Assembler messages: BUILDSTDERR: /tmp/sdma_v4_0-3f5dad.s:11941: Error: selected processor does not support `bfc w0,#1,#5' BUILDSTDERR: clang-9: error: assembler command failed with exit code 1 (use -v to see invocation) BUILDSTDERR: make[4]: *** [scripts/Makefile.build:280: drivers/gpu/drm/amd/amdgpu/sdma_v4_0.o] Error 1 BUILDSTDERR: make[4]: *** Waiting for unfinished jobs.... CC [M] drivers/gpu/drm/amd/amdgpu/gfx_v10_0.o BUILDSTDERR: make[3]: *** [scripts/Makefile.build:497: drivers/gpu/drm/amd/amdgpu] Error 2 BUILDSTDERR: make[3]: *** Waiting for unfinished jobs....

@nathanchance
Copy link
Member

I can reproduce this on mainline with your configuration files. I am running creduce just to try and narrow down the problematic part of the file as well as see if this is a configuration issue (since I have never seen this error with defconfig, allmodconfig, or allyesconfig).

@nathanchance
Copy link
Member

creduce spits out:

❯ cat sdma_v4_0.i
a, b, d;
c() { return a > 1 ? 1 ? 1 ? a - 2 ?: a ? 2 : 1 : 1 : b : 0; }
e(f) {
  int g = __builtin_constant_p(d) ? 1 ? h() : 0 : c();
  f = f & ~0x0000003EL | 62 & g << 1;
  return f;
}

Doing a quick Google search shows that maybe bfc is only available on ARMv8.2+? Adding -march=armv8.2-a resolves this error. I will try to do a config bisect to see what exactly which config tickles this.

@nickdesaulniers nickdesaulniers added the [ARCH] arm64 This bug impacts ARCH=arm64 label Sep 24, 2019
@nathanchance
Copy link
Member

Looks like the reason we have never caught this is somehow KASAN prevents it and allmodconfig/allyesconfig enables KASAN + AMDGPU, while defconfig enables neither. CONFIG_DRM_AMDGPU + defconfig is enough to reproduce this.

That creduce example is fine if -fsanitize=kernel-address is added to the clang invocation.

This appears to be a regression between LLVM 7 and 8, as LLVM 7.1.0 compiles this translation unit with that configuration just fine but not LLVM 8.0.1. git bisect turns up:

91549ed15f97f21de8770196e66da3d228820cdc is the first bad commit
commit 91549ed15f97f21de8770196e66da3d228820cdc
Author: Bill Wendling <isanbard@gmail.com>
Date:   Tue Nov 20 23:24:16 2018 +0000

    Reinstate 347294 with a fix for the failures.
    
    EvaluateAsInt() is sometimes called in a constant context. When that's the
    case, we need to specify it as so.
    
    llvm-svn: 347364

 clang/include/clang/AST/Expr.h                     |  13 +-
 clang/lib/AST/ASTImporter.cpp                      |   2 +-
 clang/lib/AST/Expr.cpp                             |  22 ++--
 clang/lib/AST/ExprConstant.cpp                     | 140 ++++++++++++++-------
 clang/lib/Analysis/CFG.cpp                         |  19 +--
 clang/lib/CodeGen/CGBuiltin.cpp                    |  54 ++++++--
 clang/lib/CodeGen/CGDebugInfo.cpp                  |   6 +-
 clang/lib/CodeGen/CGExprScalar.cpp                 |  11 +-
 clang/lib/CodeGen/CGOpenMPRuntime.cpp              |  11 +-
 clang/lib/CodeGen/CGStmt.cpp                       |   4 +-
 clang/lib/CodeGen/CGStmtOpenMP.cpp                 |   6 +-
 clang/lib/CodeGen/CodeGenFunction.cpp              |   5 +-
 clang/lib/Sema/AnalysisBasedWarnings.cpp           |   7 +-
 clang/lib/Sema/SemaCast.cpp                        |   5 +-
 clang/lib/Sema/SemaChecking.cpp                    |  47 ++++---
 clang/lib/Sema/SemaDecl.cpp                        |   6 +-
 clang/lib/Sema/SemaDeclCXX.cpp                     |   2 +
 clang/lib/Sema/SemaExpr.cpp                        |  69 ++++++----
 clang/lib/Sema/SemaInit.cpp                        |   5 +-
 clang/lib/Sema/SemaOpenMP.cpp                      |  47 ++++---
 clang/lib/Sema/SemaOverload.cpp                    |   2 +-
 clang/lib/Sema/SemaStmt.cpp                        |   5 +-
 clang/lib/Sema/SemaStmtAsm.cpp                     |   5 +-
 clang/lib/Sema/SemaTemplateDeduction.cpp           |   4 +
 clang/lib/Sema/SemaType.cpp                        |   4 -
 .../Checkers/BuiltinFunctionChecker.cpp            |   5 +-
 .../Checkers/CheckSecuritySyntaxOnly.cpp           |   5 +-
 .../Checkers/MallocOverflowSecurityChecker.cpp     |   9 +-
 .../Checkers/NumberObjectConversionChecker.cpp     |   5 +-
 clang/lib/StaticAnalyzer/Core/ExprEngine.cpp       |   3 -
 clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp      |   5 +-
 clang/lib/StaticAnalyzer/Core/SValBuilder.cpp      |   4 +-
 clang/test/Analysis/builtin-functions.cpp          |   6 +-
 clang/test/Sema/builtins.c                         |   9 ++
 clang/test/SemaCXX/compound-literal.cpp            |   4 +-
 35 files changed, 362 insertions(+), 194 deletions(-)

% git bisect log
# bad: [19a71f6bdf2dddb10764939e7f0ec2b98dba76c9] Merging r360861, with an additional change to also add the PPC64_OPD1 and PPC64_OPD2 lines to the DEFINE_LIBUNWIND_PRIVATE_FUNCTION() macro, which was removed in r357640:
# good: [4856a9330ee01d30e9e11b6c2f991662b4c04b07] cmake: Use 7.1.0 for shared object version instead of 71.0
git bisect start 'llvmorg-8.0.1' 'llvmorg-7.1.0'
# good: [536176721056689d5e9041037c4a786d040e94a5] [dsymutil] Convert recursion in lookForDIEsToKeep into worklist.
git bisect good 536176721056689d5e9041037c4a786d040e94a5
# good: [d1fe437cf190c8f70c1506d9bfd4c3b9d94c405f] [InstCombine] add test for ComputeNumSignBits with shuffle; NFC
git bisect good d1fe437cf190c8f70c1506d9bfd4c3b9d94c405f
# bad: [5419a3ce123b8ba557fab1ed722946d0efef1379] Revert: Honor -fdebug-prefix-map when creating function names for the debug info.
git bisect bad 5419a3ce123b8ba557fab1ed722946d0efef1379
# good: [7716ddf18d967018c62b2c1dd20a1efda756fed9] Since ABI's now hold a process WP, they should be handed out one per process rather than keeping a single global instance.
git bisect good 7716ddf18d967018c62b2c1dd20a1efda756fed9
# bad: [20883fc20fa3d04c3f524c653f532fbd50e9f4ee] [libcxx] Use a type that is always an aggregate in variant's tests
git bisect bad 20883fc20fa3d04c3f524c653f532fbd50e9f4ee
# good: [a7b204b44f8a8bd0d6adbb1a459fc5ec29d99d8d] [PowerPC] Set the default PLT mode on OpenBSD/powerpc to Secure PLT.
git bisect good a7b204b44f8a8bd0d6adbb1a459fc5ec29d99d8d
# bad: [5cf902ccd42b59179e98409d6e9e1b5c206b9d06] [PowerPC] Do not use vectors to codegen bswap with Altivec turned off
git bisect bad 5cf902ccd42b59179e98409d6e9e1b5c206b9d06
# good: [38b12f5e70ca425c955ca4b1a564e244948a43d9] [WebAssembly] Remove unused function return types (NFC)
git bisect good 38b12f5e70ca425c955ca4b1a564e244948a43d9
# good: [efc3d1dfaae18a3da135504098174002d42cc76f] [ConstantFolding] Add support for saturating add/sub
git bisect good efc3d1dfaae18a3da135504098174002d42cc76f
# good: [8e9baa3f17c8e54906cbffbede6f9587fe01a8c3] [CodeComplete] Penalize inherited ObjC properties for auto-completion
git bisect good 8e9baa3f17c8e54906cbffbede6f9587fe01a8c3
# bad: [91549ed15f97f21de8770196e66da3d228820cdc] Reinstate 347294 with a fix for the failures.
git bisect bad 91549ed15f97f21de8770196e66da3d228820cdc
# good: [d931c135f047cd72af30cace8b14112de665d7c2] Revert "[Sanitizer] intercept setvbuf on other platforms where it is supported"
git bisect good d931c135f047cd72af30cace8b14112de665d7c2
# good: [aa52ee277078656602a2871053ff9bf8bae9ba6e] [X86] Emit a PACKUS instead of a VECTOR_SHUFFLE from LowerTRUNCATE for v16i16->v16i8.
git bisect good aa52ee277078656602a2871053ff9bf8bae9ba6e
# good: [8eb65cb25e67a2b426b94152f9c9bb3c1025b701] [NFC] Reformat availability #defines in __config
git bisect good 8eb65cb25e67a2b426b94152f9c9bb3c1025b701
# first bad commit: [91549ed15f97f21de8770196e66da3d228820cdc] Reinstate 347294 with a fix for the failures.

cc @gwelymernans

@nathanchance
Copy link
Member

To be clear, there might not be anything wrong with that commit (i.e., kernel code might be wrong) but it was working and now isn't thus the bisect.

@nickdesaulniers
Copy link
Member

Doing a quick Google search shows that maybe bfc is only available on ARMv8.2+? Adding -march=armv8.2-a resolves this error.
there might not be anything wrong with that commit

Sounds like a bug in instruction selection (why would instruction selection pick an instruction that isn't in the base ISA?). Not in @gwelymernans ' patch. But thanks for running the bisection; too bad it didn't converge on a smoking gun related to ISEL.

@nathanchance
Copy link
Member

Yes, that is what I figured but it is worth dumping everything on the table for everyone to examine!

@bwendling
Copy link

Not in @gwelymernans ' patch

A bug?! In my patch?!?! That's obvious madness.

@kbeyls
Copy link

kbeyls commented Sep 26, 2019

Indeed, 8.2 introduced the BFC assembler instruction.
However, BFC is an alias for the BFM instruction that exists in 8.0.
So - the encoding of the BFC instruction will run just fine on armv8.0.
It's just that it's only guaranteed that assemblers will recognize the BFC instruction when targeting 8.2 or above. See this extract from the ArmARM:

Applies when sf == 1 && N == 1.
BFC , #, #
is equivalent to
BFM , XZR, #(- MOD 64), #(-1)
and is the preferred disassembly when UInt(imms) < UInt(immr).

My guess is that the bug may be in LLVM's AsmPrinter, rather than any ISel. Probably it should only print BFC (instead of the equivalent BFM assembler instruction) when explicitly targeting Armv8.2 or above. When targeting before-Armv8.0 it probably should just produce the BFM equivalent.
No idea if it's going to be hard or easy to implement this...

I guess this bug may only trigger when using a specific assembler?

http://llvm.org/viewvc/llvm-project?view=revision&revision=236245 might be a relevant commit.

@nathanchance
Copy link
Member

nathanchance commented Oct 2, 2019

Apparently, Arnd filed an LLVM bug for this back in July: https://llvm.org/pr42576

He posted a workaround patch a couple of hours ago: https://lore.kernel.org/lkml/20191002120136.1777161-7-arnd@arndb.de/

@nickdesaulniers
Copy link
Member

@nickdesaulniers nickdesaulniers added [BUG] llvm A bug that should be fixed in upstream LLVM [PATCH] Accepted A submitted patch has been accepted upstream labels Oct 3, 2019
llvm-git-migration pushed a commit to llvm/llvm-project that referenced this issue Oct 3, 2019
Summary:
Fixes pr/42576.

Link: ClangBuiltLinux/linux#697

Reviewers: t.p.northover

Reviewed By: t.p.northover

Subscribers: kristof.beyls, hiraditya, llvm-commits, srhines

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D68356

llvm-svn: 373655
dtzWill pushed a commit to llvm-mirror/llvm that referenced this issue Oct 3, 2019
Summary:
Fixes pr/42576.

Link: ClangBuiltLinux/linux#697

Reviewers: t.p.northover

Reviewed By: t.p.northover

Subscribers: kristof.beyls, hiraditya, llvm-commits, srhines

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D68356

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@373655 91177308-0d34-0410-b5e6-96231b3b80d8
@nickdesaulniers
Copy link
Member

@nickdesaulniers nickdesaulniers added [FIXED][LLVM] 10 This bug was fixed in LLVM 10.0 and removed [PATCH] Accepted A submitted patch has been accepted upstream labels Oct 3, 2019
@nickdesaulniers nickdesaulniers self-assigned this Oct 3, 2019
earl pushed a commit to earl/llvm-mirror that referenced this issue Oct 3, 2019
Summary:
Fixes pr/42576.

Link: ClangBuiltLinux/linux#697

Reviewers: t.p.northover

Reviewed By: t.p.northover

Subscribers: kristof.beyls, hiraditya, llvm-commits, srhines

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D68356

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@373655 91177308-0d34-0410-b5e6-96231b3b80d8
arichardson pushed a commit to arichardson/llvm-project that referenced this issue Nov 16, 2019
Summary:
Fixes pr/42576.

Link: ClangBuiltLinux/linux#697

Reviewers: t.p.northover

Reviewed By: t.p.northover

Subscribers: kristof.beyls, hiraditya, llvm-commits, srhines

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D68356

llvm-svn: 373655
tstellar pushed a commit to llvm/llvm-project that referenced this issue Nov 25, 2019
------------------------------------------------------------------------
r373655 | nickdesaulniers | 2019-10-03 13:10:02 -0700 (Thu, 03 Oct 2019) | 16 lines

[AArch64InstPrinter] prefer bfi to bfc for < armv8.2-a

Summary:
Fixes pr/42576.

Link: ClangBuiltLinux/linux#697

Reviewers: t.p.northover

Reviewed By: t.p.northover

Subscribers: kristof.beyls, hiraditya, llvm-commits, srhines

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D68356
------------------------------------------------------------------------
ajohnson-uoregon pushed a commit to ajohnson-uoregon/clang-rewrite-only that referenced this issue Jul 17, 2022
------------------------------------------------------------------------
r373655 | nickdesaulniers | 2019-10-03 13:10:02 -0700 (Thu, 03 Oct 2019) | 16 lines

[AArch64InstPrinter] prefer bfi to bfc for < armv8.2-a

Summary:
Fixes pr/42576.

Link: ClangBuiltLinux/linux#697

Reviewers: t.p.northover

Reviewed By: t.p.northover

Subscribers: kristof.beyls, hiraditya, llvm-commits, srhines

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D68356
------------------------------------------------------------------------
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[ARCH] arm64 This bug impacts ARCH=arm64 [BUG] llvm A bug that should be fixed in upstream LLVM [FIXED][LLVM] 10 This bug was fixed in LLVM 10.0
Projects
None yet
Development

No branches or pull requests

5 participants