Page MenuHomePhabricator

nathanchance (Nathan Chancellor)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 9 2018, 11:07 PM (188 w, 5 d)

Recent Activity

Fri, May 20

nathanchance updated subscribers of D125506: [PowerPC] Implement XL compat __fnabs and __fnabss builtins..

I bisected a crash when compiling the Linux kernel to this change.

Fri, May 20, 1:04 PM · Restricted Project, Restricted Project, Restricted Project

Mon, May 16

nathanchance updated subscribers of D124726: Suggest typoed directives in preprocessor conditionals.

I see an instance of this warning in the Linux kernel due to the "Now, for unknown directives inside a skipped conditional block, we diagnose the unknown directive as a warning if it is sufficiently similar to a directive specific to preprocessor conditional blocks" part of this change:

Mon, May 16, 10:06 AM · Restricted Project, Restricted Project, Restricted Project

Thu, May 12

nathanchance added a comment to D125410: [ELF] Align the end of PT_GNU_RELRO to max-page-size instead of common-page-size.

I am seeing a failure during a two stage build of LLVM on an AArch64 host that I bisected to this change:

Thu, May 12, 6:23 PM · Restricted Project, Restricted Project

Fri, May 6

nathanchance added a comment to rGf6dff93641b2: Pedantically warn about // comments in gnu89 mode.

Ooh, thank you for that offer! Out of curiosity, do you know of any pain stemming from the strengthening of diagnostics around implicit function declarations and implicit int, or the changes we made to diagnosing K&R C functions?

Fri, May 6, 9:10 AM · Restricted Project, Restricted Project

Thu, May 5

nathanchance added a comment to rGf6dff93641b2: Pedantically warn about // comments in gnu89 mode.

It will be more disruptive than I initially realized, as the Linux kernel has SPDX markings at the top of every source file, which are required to be prefaced with // for .c files:

Yeah, that's far too disruptive for me to feel comfortable with, so I've reverted this in 1c50909f6f8ac183b82e973457522439a8856e96. Thank you for bringing this to my attention so quickly!

Thu, May 5, 3:51 PM · Restricted Project, Restricted Project
nathanchance added a comment to rGf6dff93641b2: Pedantically warn about // comments in gnu89 mode.

This triggers even without -pedantic, is that intentional?

Yes, it is also triggered by -Wcomment in addition to -pedantic

$ echo "// comment" | clang -fsyntax-only -std=gnu89 -Wall -Wextra -x c -
<stdin>:1:1: warning: // comments are not allowed in this language [-Wcomment]
// comment
^
1 warning generated.

$ echo "// comment" | clang -fsyntax-only -std=gnu89 -Wall -Wextra -pedantic -x c -
<stdin>:1:1: warning: // comments are not allowed in this language [-Wcomment]
// comment
^
<stdin>:1:11: warning: ISO C requires a translation unit to contain at least one declaration [-Wempty-translation-unit]
// comment
          ^
2 warnings generated.

vs.

$ echo "// comment" | gcc -fsyntax-only -std=gnu89 -Wall -Wextra -x c -

$ echo "// comment" | gcc -fsyntax-only -std=gnu89 -Wall -Wextra -pedantic -x c -
<stdin>:1:1: warning: C++ style comments are not allowed in ISO C90
<stdin>:1:1: note: (this will be reported only once per input file)
<stdin>:2: warning: ISO C forbids an empty translation unit [-Wpedantic]

This is going to be quite noisy for older versions of the Linux kernel, which still build with -std=gnu89.

-Wcomment is enabled by -Wmost and -Wmost is enabled by -Wall, so from that, yes, I would expect this to trigger.

Thu, May 5, 2:28 PM · Restricted Project, Restricted Project
nathanchance updated subscribers of rGf6dff93641b2: Pedantically warn about // comments in gnu89 mode.

This triggers even without -pedantic, is that intentional?

Thu, May 5, 1:17 PM · Restricted Project, Restricted Project

Fri, Apr 29

nathanchance added a comment to D124231: [RISCV] Merge addi into load/store as there is a ADD between them.

I bisected a crash while building the Linux kernel for RISC-V to this change:

Fri, Apr 29, 3:46 PM · Restricted Project, Restricted Project

Thu, Apr 28

nathanchance updated subscribers of D123720: [VPlan] Replace use of needsVectorIV with VPlan user check..

I bisected a new crash while building the Linux kernel for arm64 to this change:

Thu, Apr 28, 11:53 AM · Restricted Project, Restricted Project

Apr 21 2022

nathanchance added a comment to D123985: [ELF] Assert on invalid GOT or PLT relocations.

It looks like Fangrui already fixed this with f4a3569d0ad61907692fe10b1ffe1fa7763e9c26, thanks a lot! I can confirm that ARCH=arm64 defconfig and allmodconfig build fine now. I will test other architectures tonight and report back if there are any other issues I uncover.

Apr 21 2022, 9:04 AM · Restricted Project, Restricted Project

Apr 20 2022

nathanchance updated subscribers of D123985: [ELF] Assert on invalid GOT or PLT relocations.

This assertion triggers when building the Linux kernel for arm64:

Apr 20 2022, 7:17 PM · Restricted Project, Restricted Project

Mar 22 2022

nathanchance committed rG4e0008dcbe9f: Revert "[InstCombine] try to narrow shifted bswap-of-zext" (authored by nathanchance).
Revert "[InstCombine] try to narrow shifted bswap-of-zext"
Mar 22 2022, 5:41 PM · Restricted Project
nathanchance added a reverting change for rG9e9bda2e8f5b: [InstCombine] try to narrow shifted bswap-of-zext: rG4e0008dcbe9f: Revert "[InstCombine] try to narrow shifted bswap-of-zext".
Mar 22 2022, 5:41 PM · Restricted Project
nathanchance added a reverting change for D122166: [InstCombine] try to narrow shifted bswap-of-zext: rG4e0008dcbe9f: Revert "[InstCombine] try to narrow shifted bswap-of-zext".
Mar 22 2022, 5:41 PM · Restricted Project, Restricted Project
nathanchance added a comment to D122166: [InstCombine] try to narrow shifted bswap-of-zext.

I have reverted this in 4e0008dcbe9fce99b9727e8bbeb129efc7bf2d80 to hopefully keep our CI from going completely red. I am happy to test a new revision as needed.

Mar 22 2022, 5:41 PM · Restricted Project, Restricted Project
nathanchance added a comment to D122166: [InstCombine] try to narrow shifted bswap-of-zext.

This patch causes a crash while building the Linux kernel for arm64. A simplifier C reproducer:

Mar 22 2022, 5:24 PM · Restricted Project, Restricted Project

Mar 21 2022

nathanchance added a comment to D122189: [Clang][NeonEmitter] emit ret decl first for -Wdeclaration-after-statement.

This passes through all my ARCH=arm and ARCH=arm64 build and boot tests on next-20220321 with the instances of -Wdeclaration-after-statement resolved and no new instances of any other warnings.

Mar 21 2022, 5:00 PM · Restricted Project, Restricted Project

Mar 3 2022

nathanchance added a comment to D120926: [Mips] support "sp" named register.

I can build malta_defconfig on next-20220303 with this patch. I enabled CONFIG_DEBUG_STACKOVERFLOW and CONFIG_HARDENED_USERCOPY, which both use current_stack_pointer, and did not see any warnings/errors.

Mar 3 2022, 1:34 PM · Restricted Project, Restricted Project

Feb 28 2022

nathanchance added a comment to D120192: [DAG] Attempt to fold bswap(shl(x,c)) -> zext(bswap(trunc(shl(x,c-bw/2)))).

@nathanchance Please can you confirm if rGfadd20f80d69 fixes your build?

Feb 28 2022, 8:01 AM · Restricted Project

Feb 25 2022

nathanchance added a comment to D120192: [DAG] Attempt to fold bswap(shl(x,c)) -> zext(bswap(trunc(shl(x,c-bw/2)))).

This patch causes a failed assertion when building the arm64 Linux kernel.

Feb 25 2022, 12:55 PM · Restricted Project

Feb 18 2022

nathanchance added a comment to D118840: [ELF] Support (TYPE=<value>) to customize the output section type.

If the section name '.plt' is special in some way, I'd be happy to prepare a kernel patch that renames these into something more suitable.

Feb 18 2022, 7:38 AM · Restricted Project, Restricted Project

Feb 17 2022

nathanchance updated subscribers of D118840: [ELF] Support (TYPE=<value>) to customize the output section type.

I have not done a bisect but it seems likely that this change breaks linking modules for the arm64 Linux kernel, is this expected?

Feb 17 2022, 5:22 PM · Restricted Project, Restricted Project

Feb 16 2022

nathanchance added a comment to D119978: [libc] Use '+' constraint on inline assembly.

@nathanchance could you take a look if this fixes the problem from https://github.com/llvm/llvm-project/issues/53391 for you? I was not able to reproduce this error after the workaround

Feb 16 2022, 4:28 PM · Restricted Project

Feb 15 2022

nathanchance added a comment to D118663: [AArch64] Adds SUBS and ADDS instructions to the MIPeepholeOpt..

The latest revision passes all of my AArch64 Linux kernel builds with assertions enabled and boots on bare metal, thank you for fixing all the issues!

Feb 15 2022, 10:30 AM · Restricted Project

Feb 13 2022

nathanchance added a comment to D118663: [AArch64] Adds SUBS and ADDS instructions to the MIPeepholeOpt..

Thank you for the quick fix! Unfortunately, I still an assertion failure with this patch while building at least allnoconfig and allmodconfig + ThinLTO kernels, albeit a different one. See below and let me know if there are any problems with reproducing:

Feb 13 2022, 9:04 PM · Restricted Project
nathanchance updated subscribers of D118663: [AArch64] Adds SUBS and ADDS instructions to the MIPeepholeOpt..

I have reverted this in 22eb1dae3fb20ca8ada865de1d95baab0e08a060, as it causes assertion failures when building the Linux kernel, which has caused our CI to go red:

Feb 13 2022, 9:41 AM · Restricted Project
nathanchance added a reverting change for rGaf45d0fd94b2: [AArch64] Adds SUBS and ADDS instructions to the MIPeepholeOpt.: rG22eb1dae3fb2: Revert "[AArch64] Adds SUBS and ADDS instructions to the MIPeepholeOpt.".
Feb 13 2022, 9:41 AM
nathanchance committed rG22eb1dae3fb2: Revert "[AArch64] Adds SUBS and ADDS instructions to the MIPeepholeOpt." (authored by nathanchance).
Revert "[AArch64] Adds SUBS and ADDS instructions to the MIPeepholeOpt."
Feb 13 2022, 9:41 AM
nathanchance added a reverting change for D118663: [AArch64] Adds SUBS and ADDS instructions to the MIPeepholeOpt.: rG22eb1dae3fb2: Revert "[AArch64] Adds SUBS and ADDS instructions to the MIPeepholeOpt.".
Feb 13 2022, 9:41 AM · Restricted Project

Feb 7 2022

nathanchance added a comment to D110869: [X86] Implement -fzero-call-used-regs option.

All built+boot. I know @nathanchance pointed out an issue with some already complex code, but I'm of the opinion that fn should be attributed in kernel sources.

Feb 7 2022, 4:52 PM · Restricted Project, Restricted Project

Feb 3 2022

nathanchance added a comment to D110869: [X86] Implement -fzero-call-used-regs option.

It looks like _paravirt_ident_64() is the problematic function. This diff on top of v5.17-rc2 allows me to boot:

Feb 3 2022, 4:50 PM · Restricted Project, Restricted Project
nathanchance added a comment to D110869: [X86] Implement -fzero-call-used-regs option.

This diff allows me to boot on bare metal as of v5.17-rc2:

diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 6aef9ee28a39..8ee176dac669 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -125,6 +125,7 @@ obj-$(CONFIG_DEBUG_NMI_SELFTEST) += nmi_selftest.o

 obj-$(CONFIG_KVM_GUEST)                += kvm.o kvmclock.o
 obj-$(CONFIG_PARAVIRT)         += paravirt.o
+CFLAGS_paravirt.o += -fzero-call-used-regs=skip
 obj-$(CONFIG_PARAVIRT_SPINLOCKS)+= paravirt-spinlocks.o
 obj-$(CONFIG_PARAVIRT_CLOCK)   += pvclock.o
 obj-$(CONFIG_X86_PMEM_LEGACY_DEVICE) += pmem.o

I have uploaded the config used, the preprocessed file, and the "good" object (with the following diff) and the "bad" object (without the above diff) here: https://github.com/nathanchance/bug-files/tree/052a31e6d94c1b349cf6f3128087944444dace24/D110869

If there is any more information I can give, please let me know!

Thanks for the information!

Could you test this patch for me? (Applied over the patch in this review.)

diff --git a/llvm/lib/CodeGen/PrologEpilogInserter.cpp b/llvm/lib/CodeGen/PrologEpilogInserter.cpp
index 968a14548813..46ae48bd6a3c 100644
--- a/llvm/lib/CodeGen/PrologEpilogInserter.cpp
+++ b/llvm/lib/CodeGen/PrologEpilogInserter.cpp
@@ -1217,7 +1217,8 @@ void PEI::insertZeroCallUsedRegs(MachineFunction &MF) {
             continue;

           MCRegister Reg = MO.getReg();
-          if ((MO.isDef() || MO.isUse()) && AllocatableSet[Reg])
+          if (AllocatableSet[Reg] && !MO.isImplicit() &&
+              (MO.isDef() || MO.isUse()))
             UsedRegs.set(Reg);
         }
Feb 3 2022, 2:28 PM · Restricted Project, Restricted Project
nathanchance added a comment to D110869: [X86] Implement -fzero-call-used-regs option.

This diff allows me to boot on bare metal as of v5.17-rc2:

Feb 3 2022, 12:23 PM · Restricted Project, Restricted Project
nathanchance added a comment to D110869: [X86] Implement -fzero-call-used-regs option.

The latest revision allows me to boot a kernel in QEMU now but that same kernel does not boot on bare metal. I'll see if I can narrow down the problematic translation unit with Nick's subdir-ccflags-y trick.

Feb 3 2022, 9:07 AM · Restricted Project, Restricted Project

Jan 27 2022

nathanchance abandoned D117717: [clang] Ignore -fconserve-stack.

As discussed, this is not the right fix for the issue. I have just gone ahead and told the original reporter to use scan-build with the --use-cc=clang flag to avoid these issues.

Jan 27 2022, 7:44 PM · Restricted Project

Jan 19 2022

nathanchance added a comment to D117717: [clang] Ignore -fconserve-stack.

Assuming scan-build is clang/tools/scan-build, which is in tree and can be fixed instead.

Jan 19 2022, 3:10 PM · Restricted Project
nathanchance added a comment to D117717: [clang] Ignore -fconserve-stack.

Clang has a long list of ignored options but most were added during the catch-up phase when Clang strove to be a competitor compiler.
I think for new ignored options there needs to be some higher standard.
From Debian Code Search https://codesearch.debian.net/search?q=fconserve-stack&literal=0&page=2 and internal code search, I believe this is a fairly obscure option.
Shall we fix the projects instead?

Jan 19 2022, 2:03 PM · Restricted Project
nathanchance requested review of D117717: [clang] Ignore -fconserve-stack.
Jan 19 2022, 1:18 PM · Restricted Project

Jan 15 2022

nathanchance added a comment to D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default.

I can reduce all of these down for you and/or I can start an email thread with the objtool maintainers to see if there is a way to fix or avoid these warnings on the objtool side and include you in that discussion, if LLVM is not really doing anything wrong here. I am by no means an expert in this area and I don't want to delay this anymore but I want to avoid regressing our builds, as objtool regularly helps us spot compiler bugs. Perhaps @nickdesaulniers has some other thoughts?

LLVM is not doing anything wrong here. The issue is that once we have UB, LLVM is not required to lay out the functions nicely. For example, issue #53118 is just a "nice to fix", it's not a bug.
On the other hand, I understand that fixing objtool is likely impossible if we consider this UB thing as assembly doesn't have enough information to distinguish between a compiler bug and a UB case. I just don't know how frequent and/or useful these warnings are.

I don't think we should hold this patch anymore as it's not wrong and the side-effects are understood. We can and should try to reduce those kernel warnings to zero, but we cannot put all that burden on this patch's author.

Jan 15 2022, 3:09 PM · Restricted Project

Jan 14 2022

nathanchance updated subscribers of D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default.

@nathanchance Hi, I analyzed all four warnings.

Jan 14 2022, 12:14 PM · Restricted Project

Jan 12 2022

nathanchance added a comment to D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default.

@nathanchance

I tried to reproduce the last warning (intelfbhw_validate_mode), but I failed to produce it.
I think my reproducer is correct, but it does not make any warning.
Can you tell me which part was wrong?

clang -O2 -flto=thin -fsanitize=integer-divide-by-zero -c -o intelfb.o intelfb.i
ld.lld -m elf_x86_64 -r -o intelfb.lto.o --whole-archive intelfb.o
objtool orc generate --module --no-fp --no-unreachable --uaccess --mcount intelfb.lto.o

I use these commands, and I attached the intelfb.i file.

Jan 12 2022, 9:15 AM · Restricted Project

Jan 10 2022

nathanchance added a comment to D116766: [SCEV] Sequential/in-order `UMin` expression.

f62f47f5e1f641b41d3b7d593c058ebec2883534 hides the assertion failure for me.

Jan 10 2022, 7:18 PM · Restricted Project
nathanchance added a comment to D116766: [SCEV] Sequential/in-order `UMin` expression.

Just in case it is something different than the Chromium report, I see an assertion failure while building the Linux kernel. Reduced reproducer:

Jan 10 2022, 5:48 PM · Restricted Project

Jan 7 2022

nathanchance added a comment to D116059: [Clang][CFG] check children statements of asm goto.

The two false positive warnings in the kernel are fixed with this patch and there were no new warnings introduced.

Jan 7 2022, 8:08 AM · Restricted Project

Dec 29 2021

nathanchance added a comment to D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default.

@hyeongyukim I hand reduced a couple of the translation units that I see issues in so apologies if they are a little verbose.

Dec 29 2021, 4:26 PM · Restricted Project

Dec 28 2021

nathanchance added a comment to D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default.

@hyeongyukim I am currently offline for the evening but it seems like my reduction might have been too aggressive. It looks like this code comes from ravb_set_gti() (https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/renesas/ravb_main.c?h=v5.16-rc7#n2480), which checks that rate is not zero before using it as a divisor. I will see if I can get a reproducer without any undefined behavior such as this.

Dec 28 2021, 6:38 PM · Restricted Project

Dec 23 2021

nathanchance committed rGbe8180af5854: [clang][driver] Warn when '-mno-outline-atomics' is used with a non-AArch64… (authored by nathanchance).
[clang][driver] Warn when '-mno-outline-atomics' is used with a non-AArch64…
Dec 23 2021, 11:49 AM
nathanchance closed D116128: [clang][driver] Warn when '-mno-outline-atomics' is used with a non-AArch64 triple.
Dec 23 2021, 11:49 AM · Restricted Project

Dec 22 2021

nathanchance updated the diff for D116128: [clang][driver] Warn when '-mno-outline-atomics' is used with a non-AArch64 triple.
  • Move test from x86-outline-atomics.c to unsupported-outline-atomics.c (Marco).
Dec 22 2021, 8:36 AM · Restricted Project

Dec 21 2021

nathanchance added inline comments to D116128: [clang][driver] Warn when '-mno-outline-atomics' is used with a non-AArch64 triple.
Dec 21 2021, 3:00 PM · Restricted Project
nathanchance added a comment to D116128: [clang][driver] Warn when '-mno-outline-atomics' is used with a non-AArch64 triple.

If there are more qualified people to review and accept this, please add them, I was unsure of who else to add.

Dec 21 2021, 1:20 PM · Restricted Project
nathanchance requested review of D116128: [clang][driver] Warn when '-mno-outline-atomics' is used with a non-AArch64 triple.
Dec 21 2021, 1:20 PM · Restricted Project

Dec 13 2021

nathanchance added a comment to D115635: [llvm-objcopy] Fix handling of MIPS64 little endian files.

This resolves the Linux kernel build error, thank you for the quick fix!

Dec 13 2021, 10:21 AM · Restricted Project

Dec 1 2021

nathanchance added a comment to D114848: [Analysis] Ignore casts and unary ops for uninitialized values.

I tested the problematic Linux kernel configuration that uncovered this issue with this patch and the issue was resolved. I tested an x86_64 allmodconfig build and did not see any warnings.

Dec 1 2021, 9:00 AM · Restricted Project

Nov 8 2021

nathanchance added a comment to D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default.

Prior to the latest revert (fd9b099906c61e46574d1ea2d99b973321fe1d21), the Linux kernel's binary verifier (objtool) points out an issue when building with ThinLTO and -fsanitize=integer-divide-by-zero. I have no idea if this is an issue with the tool or this series. A simplified reproducer:

Nov 8 2021, 2:27 PM · Restricted Project

Nov 6 2021

nathanchance added a comment to D111199: [Clang][LLVM][Attr] support btf_type_tag attribute.

I bisected a crash in the Linux kernel down to the reland commit (and it is not fixed as of https://github.com/llvm/llvm-project/commit/2249ecee8d9a6237f51485bd39f01ba031575987):

Nov 6 2021, 4:36 PM · Restricted Project, Restricted Project, Restricted Project

Nov 3 2021

nathanchance added a comment to D110867: X86InstrInfo: Support immediates that are +1/-1 different in optimizeCompareInstr.

I too saw issues with this patch with PGO (I was in the middle of writing up a reproducer when I saw Hans comment). My symptom was that stage 3 check-llvm would not pass, whereas stage 2 would. This version of the patch on top of 7277d2e1c86bf4d75321efc1195d88ade4bedfa1 does not have that same issue.

Nov 3 2021, 11:18 AM · Restricted Project

Oct 27 2021

nathanchance updated subscribers of D112600: [ARM] Use hardware TLS register in Thumb2 mode when -mtp=cp15 is passed.

I tested this against next-20211027, where it resolved the build error and boots in QEMU master without any visible issues. Disassembly between GCC and clang seems to be good. I'll leave it up to @nickdesaulniers and @peter.smith to approve, as I am not very familiar with TableGen.

Oct 27 2021, 7:48 AM · Restricted Project, Restricted Project

Oct 26 2021

nathanchance added a comment to D110869: [X86] Implement -fzero-call-used-regs option.

check-llvm does not pass with this revision nor does it resolve the boot failure that Nick mentioned, just in case that was the intention of the revision :) I ran the kernel through gdb and it still does not appear to get to start_kernel.

Oct 26 2021, 8:44 AM · Restricted Project, Restricted Project

Oct 1 2021

nathanchance added a comment to D110656: [clang][Sema] Warn on uninitialized array elments.

I can test this on the Linux kernel tonight and report the results tomorrow morning.

Oct 1 2021, 5:50 PM · Restricted Project

Sep 30 2021

nathanchance added a comment to D110656: [clang][Sema] Warn on uninitialized array elments.

I can test this on the Linux kernel tonight and report the results tomorrow morning.

Sep 30 2021, 10:54 AM · Restricted Project
nathanchance added a comment to D108003: [Clang] Extend -Wbool-operation to warn about bitwise and of bools with side effects.

As far as I am aware, this iteration of the patch has no instances in the Linux kernel now. The instances I found with the first iteration of the patch only had a right hand side with side effects, not both sides, so this warning is effectively a no-op now (not that there won't be instances in the future). If any happen to show up, I will send fixes for them at that time. In other words, LGTM.

Sep 30 2021, 10:30 AM · Restricted Project

Sep 23 2021

nathanchance added a comment to D108607: [RISCV] Optimize (add (mul x, c0), c1).

Thanks, I can confirm that commit fixes it!

Sep 23 2021, 10:59 PM · Restricted Project
nathanchance added a comment to D108607: [RISCV] Optimize (add (mul x, c0), c1).

This commit causes an infinite loop when compiling the Linux kernel:

Sep 23 2021, 7:49 AM · Restricted Project

Aug 25 2021

nathanchance updated subscribers of D106056: [CVP] processSwitch: Remove default case when switch cover all possible values..
Aug 25 2021, 10:38 AM · Restricted Project

Aug 24 2021

nathanchance added a comment to D106056: [CVP] processSwitch: Remove default case when switch cover all possible values..

Thank you for all of the insight so far, I can confirm that adding -mllvm -trap-unreachable to KBUILD_{C,LD}FLAGS does resolve the issue. I have brought up this discussion with the objtool maintainers here and I will report back here depending on what they say; hopefully you do not mind holding off on merging the patch until there is a response.

Aug 24 2021, 1:09 PM · Restricted Project

Aug 23 2021

nathanchance added a comment to D106056: [CVP] processSwitch: Remove default case when switch cover all possible values..

I do not think that is the issue here. Adding -mllvm -trap-unreachable to the kernel's build flags does not silence objtool nor allows the kernel to boot. I switched over to CONFIG_UNWINDER_FRAME_POINTER instead of CONFIG_UNWINDER_ORC so objtool should only be running checks, not doing any sort of binary modifications, and I see the same warning. If the warning is not valid, then I can bring that up to the tool's author but it seems valid, given that it is pointing to the exact function that @junparser already identified as problematic.

Aug 23 2021, 5:23 PM · Restricted Project
nathanchance added a comment to D106056: [CVP] processSwitch: Remove default case when switch cover all possible values..

All I can go off of at this point is the objtool warning so here is a reduced C program that reproduces with this patch:

Aug 23 2021, 2:35 PM · Restricted Project

Aug 20 2021

nathanchance added a comment to D106056: [CVP] processSwitch: Remove default case when switch cover all possible values..

I will be offline until Sunday at the earliest, I can try to get a reproducer of the issue then. Not sure if @samitolvanen would be able to provide one sooner.

Aug 20 2021, 10:42 AM · Restricted Project

Aug 19 2021

nathanchance added a comment to D108003: [Clang] Extend -Wbool-operation to warn about bitwise and of bools with side effects.

I am still running a series of builds against the Linux kernel but I already see one instance of this warning where the suggestion would change the meaning of the code in an incorrect way:

Aug 19 2021, 5:01 PM · Restricted Project
nathanchance added a comment to D106056: [CVP] processSwitch: Remove default case when switch cover all possible values..

@junparser I am not convinced this is a bug in objtool. @spatel 's comment makes me wonder if one of the other switch cases is becoming just return false because LTO allows LLVM to see that one of the if (max_contiguous < ...) checks is always true then this transform removes both that case and the default case because they are equivalent, which seems to be inline with what objtool is warning about. I am more than happy to test follow up versions of this patch as I am subscribed, feel free to ping me as well.

Aug 19 2021, 11:55 AM · Restricted Project

Aug 18 2021

nathanchance updated subscribers of D106056: [CVP] processSwitch: Remove default case when switch cover all possible values..

I apologize that I do not have a ton of information (I am happy to provide whatever would be useful) but this patch causes an issue with booting an x86_64_defconfig Linux kernel build with ThinLTO (initially reported here: https://github.com/ClangBuiltLinux/linux/issues/1440).

Aug 18 2021, 12:45 PM · Restricted Project

Aug 16 2021

nathanchance committed rG9ed4a94d6451: [clang] Expose unreachable fallthrough annotation warning (authored by nathanchance).
[clang] Expose unreachable fallthrough annotation warning
Aug 16 2021, 5:16 PM
nathanchance closed D107933: [clang] Expose unreachable fallthrough annotation warning.
Aug 16 2021, 5:16 PM · Restricted Project
nathanchance updated the diff for D107933: [clang] Expose unreachable fallthrough annotation warning.
  • Update commit message (forgot to use arc diff --edit --verbatim) [David]
Aug 16 2021, 1:42 PM · Restricted Project

Aug 14 2021

nathanchance added a comment to D108003: [Clang] Extend -Wbool-operation to warn about bitwise and of bools with side effects.

I have sent the following patches for the Linux kernel. These were the only instances of the warning that I found across the code base in all of the configurations that we usually test so thank you for the heads up so I could send fixes before this lands.

Aug 14 2021, 6:03 PM · Restricted Project

Aug 13 2021

nathanchance added inline comments to D107933: [clang] Expose unreachable fallthrough annotation warning.
Aug 13 2021, 3:30 PM · Restricted Project
nathanchance updated the diff for D107933: [clang] Expose unreachable fallthrough annotation warning.
  • -Wimplicit-fallthrough-unreachable -> -Wunreachable-code-fallthrough, enabled under -Wunreachable-code (Dávid, Aaron).
Aug 13 2021, 3:29 PM · Restricted Project
nathanchance added a comment to D108003: [Clang] Extend -Wbool-operation to warn about bitwise and of bools with side effects.

FWIW, all three of @nathanchance's detected lines look like good true positives to me: even if they're not bugs, they all look like places the programmer meant to write && and only wrote & by accident. The third one might even be a bug bug, since it's doing essentially (bounds-check offset_1) & (pointer-math-on offset_1).

Aug 13 2021, 1:36 PM · Restricted Project
nathanchance added a comment to D107933: [clang] Expose unreachable fallthrough annotation warning.

Yes, something like that, plus I think you want put UnreachableCodeFallthrough into group UnreachableCode as well.

Aug 13 2021, 11:11 AM · Restricted Project
nathanchance added a comment to D108003: [Clang] Extend -Wbool-operation to warn about bitwise and of bools with side effects.

With several Linux kernel defconfig builds, I see the following instances of the warning:

Aug 13 2021, 9:36 AM · Restricted Project

Aug 12 2021

nathanchance added a comment to D107933: [clang] Expose unreachable fallthrough annotation warning.

So something more along the lines of

Aug 12 2021, 2:04 PM · Restricted Project
nathanchance added a comment to D107933: [clang] Expose unreachable fallthrough annotation warning.

GCC does not warn (with common -Wall) for this case, right? I think Clang should not as well.

Aug 12 2021, 1:03 PM · Restricted Project

Aug 11 2021

nathanchance requested review of D107933: [clang] Expose unreachable fallthrough annotation warning.
Aug 11 2021, 3:23 PM · Restricted Project

Aug 8 2021

nathanchance added a comment to D106667: SROA: Enhance speculateSelectInstLoads.

This patch causes an assertion failure while building the MIPS Linux kernel:

Aug 8 2021, 1:13 PM · Restricted Project

Jul 28 2021

nathanchance committed rG5060224d9eed: [test] Fix tools/gold/X86/comdat-nodeduplicate.ll on non-X86 hosts (authored by nathanchance).
[test] Fix tools/gold/X86/comdat-nodeduplicate.ll on non-X86 hosts
Jul 28 2021, 9:57 PM
nathanchance closed D107020: [test] Fix tools/gold/X86/comdat-nodeduplicate.ll on non-X86 hosts.
Jul 28 2021, 9:57 PM · Restricted Project
nathanchance requested review of D107020: [test] Fix tools/gold/X86/comdat-nodeduplicate.ll on non-X86 hosts.
Jul 28 2021, 6:55 PM · Restricted Project
nathanchance added a comment to D106754: [RISCV] Restrict performANY_EXTENDCombine to prevent an infinite loop..

Can this be cherry-picked to release/13.x?

Jul 28 2021, 9:34 AM · Restricted Project

Jul 25 2021

nathanchance added a comment to D106754: [RISCV] Restrict performANY_EXTENDCombine to prevent an infinite loop..

I can confirm that an ARCH=riscv allmodconfig Linux kernel can now successfully build.

Jul 25 2021, 3:34 PM · Restricted Project

Jul 23 2021

nathanchance added a comment to D104581: [RISCV] Add DAG combine to detect opportunities to replace (i64 (any_extend (i32 X)) with sign_extend..

This patch causes an infinite loop while compiling the RISC-V Linux kernel's allmodconfig target. A simplified reproducer:

Jul 23 2021, 7:18 PM · Restricted Project

Jul 11 2021

nathanchance added a comment to D105757: [SystemZ] Bugfix for the 'N' code for inline asm operand..

Thanks, I can confirm this fixes the crash that I saw and does not regress the build or boot in other ways.

Jul 11 2021, 10:02 PM · Restricted Project

Jul 9 2021

nathanchance added a comment to D105502: [SystemZ] Support the 'N' code for the odd register in inline-asm..

I see the following crash when compiling the -next Linux kernel, which contains https://git.kernel.org/next/linux-next/c/4516f355c55f6da231c494c6d2be7d863d02f13c, which mentions this commit directly:

Jul 9 2021, 11:54 AM · Restricted Project

Jun 27 2021

nathanchance committed rG4ae0ab095bf9: [BitCode] Add noprofile to getAttrFromCode() (authored by nathanchance).
[BitCode] Add noprofile to getAttrFromCode()
Jun 27 2021, 12:02 PM
nathanchance closed D104995: [BitCode] Add noprofile to getAttrFromCode().
Jun 27 2021, 12:01 PM · Restricted Project
nathanchance requested review of D104995: [BitCode] Add noprofile to getAttrFromCode().
Jun 27 2021, 10:55 AM · Restricted Project

Jun 24 2021

nathanchance added a comment to D104836: [PowerPC] Combine 64-bit bswap(load) without LDBRX.

I can confirm that this reduces the stack size of the function in the Linux kernel that prompted the report:

Jun 24 2021, 11:37 AM · Restricted Project

Jun 21 2021

nathanchance added a reverting change for rGbb1dc876ebb8: [LoopDeletion] Handle Phis with similar inputs from different blocks: rGf52666985d70: Revert "[LoopDeletion] Handle Phis with similar inputs from different blocks".
Jun 21 2021, 10:33 AM
nathanchance added a comment to D103959: [LoopDeletion] Handle Phis with similar inputs from different block.

@nathanchance - I'd suggest reverting until @mkazantsev can investigate. He's offline for the next few hours due to time zones.

Jun 21 2021, 10:33 AM · Restricted Project
nathanchance committed rGf52666985d70: Revert "[LoopDeletion] Handle Phis with similar inputs from different blocks" (authored by nathanchance).
Revert "[LoopDeletion] Handle Phis with similar inputs from different blocks"
Jun 21 2021, 10:33 AM