- User Since
- Apr 13 2018, 4:23 PM (103 w, 4 d)
- tighten up condition, eagerly emit diag, test with kernel before pushing
- emit warning just for CallExprs
Note your test case is related to -Wunused-value, mine is -Wunused-result (via the use of __attribute__((warn_result_unused)) which we should always warn about.
Mon, Apr 6
ah, using this to build a Linux kernel triggers tons of warnings. So the behavior of GCC is even more subtle than captured in the test cases here.
- actually compile test the change...
interdiff LGTM, please wait for @efriedma 's feedback, too before landing. Thanks for this improvement!
- touch fewer lines as per Bill
- simplify function
Thanks for the citation. I verified in https://static.docs.arm.com/ddi0487/fa/DDI0487F_a_armv8_arm.pdf pdf page 4449 that <Rt> must be even-numbered and not R14.
Fri, Apr 3
- add period to end of comment.
- update comment- update comment- update comment- update comment- update comment- update comment- update comment- update comment- update comment
- drop verifier checks, fix typos
Thu, Apr 2
- redo everything
- forgot to rebuild all of llvm before run of update_llc_test_checks.py
FWIW: this is the current state of the patch: https://github.com/ClangBuiltLinux/llvm-project/commit/21a14be632072013a2800f8747154ca1a271d3b6
Ok, everything is green now, kernel boots. (There's probably more verification I can do, too).
Ok, I think I have a fix in hand; it's still regressing the existing test, but I think it's more along the lines of the above changes regarding Address of block that was removed by CodeGen vs Block address taken and I don't have the below diff in this new branch I'm working out of yet, but need to double check. It is now producing valid predecessors and successors, and verifies that.
From the mailing list:
Just a status update; yesterday I found/observed how the phi nodes get emitted from selectiondag. This morning I've found a fix for the mangled phi node, but that regresses other phi nodes in the test case. I think I understand why, and will continue to investigate further, but I'm going for a smoke to contemplate. I didn't smoke before this bug.
Tue, Mar 31
Anyways, tests are now green for me, and I'm happy with the patch. Thanks for all of the work that went into this @bd1976llvm . It doesn't really matter to me which way we resolve the above comment about echo.
Mon, Mar 30
An INLINEASM_BR edge should count as a predecessor, yes; if that edge is missing, something went wrong.
Fri, Mar 27
Thanks for the patch!
Patch looks good with everything I was able to throw at it; let's get that test fixed up so we don't land new tests that are red, and we should be good to go here.
Cool thanks for addition; finding and fixing things in code review is better than having patches reverted.
Wed, Mar 25
Tue, Mar 24
Fri, Mar 20
So I rebased this, wrote tests that pass, then tried it on my kernel and...it didn't lower the warning count any. So I probably won't be pursing this revision further. Seems that most of our out of line assembly uses .section/.pushsection for progbits (or unspecified, which I guess defaults to progbits), but is all compiled with -Wa,-gdwarf-2.
Hello, we're hitting this issue when assembling the Linux kernel with Clang (https://github.com/ClangBuiltLinux/linux/issues/716). I'd be happy to dust this off/rebase and add tests. @dim would you prefer to do it or shall I?
I only use arc for:
- pushing changes to phabricator via arc diff. When I make changes in response to code review, I git commit -a --amend, or squash multiple patches all into one, then run arc.
- fetching changes from phabricator via arc patch D<12345> to either commit, or help test.
Thu, Mar 19
When I merge changes, I do:
From a testing perspective, LGTM. I was able to build+boot a 4.4 aarch64 Linux kernel built with clang with this patch applied! Thank you.
Thanks for the modifications, I'm happy to give this a shot and report back, if you would kindly mind waiting to submit this on that report? I'll take care of this now.
Wed, Mar 18
Tue, Mar 17
@sdesmalen sorry, would you mind re-reviewing. I'm not comfortable landing with the previous version being reviewed and not the current one.
- fix 'U' case
Mon, Mar 16
Hi @falstaff84 , do you need help committing this?
Apologies for the revert, but thank you @thakis for it. If you need help testing an updated V2, I'd be happy to help.
Hello! Thanks for this patch. In release builds, I'm seeing lots of warnings that:
Fri, Mar 13
Hello, it seems that this patch is causing a compiler crash for us when building a 4.4 Linux kernel.
It appears to cause a crash during inlining, though I just run opt -inline <module before inlining from -O2> I don't observe the crash. I assume there's some analysis pass I need to run first? opt -instsimplify -inline also didn't seem to work, though opt -O2 does. See link above for steps to reproduce and most concise reproducer I could come up with.
- add support for compile time arrays