- User Since
- Nov 19 2018, 1:12 AM (134 w, 4 d)
Tue, Jun 15
Updated to address (some) review comments, clang-tidy warnings and taking the Succ->getSinglePredecessor() condition into account to avoid duplication of instructions.
Thanks @bjope for the input. I do share your concern. While I think we all agree that SimplifyCFG is the right place for this kind of transformation the steps that would follow acceptance and landing of the current patch do not seem to directly lead to being able to remove the unconditional assume elimination from CodeGenPrepare. At least not without possibly introducing other degradation as there may be other transformation that have run in between and now put assumes in inconvenient places. As pointed out by @bjope.
Tue, Jun 8
Is this the way we want to integrate into SimplifyCFG? Currently only tested with the supplied lit-test.
Mon, Jun 7
Mon, May 31
Since there seem to be agreement on the placement in SimplifyCFG the code has been moved there.
Fri, May 28
Abandoned in favor of D103316.
Thu, May 27
May 19 2021
Apr 29 2021
Apr 28 2021
Prototype of the 'terminate debug-info for slots when they go out of liveness, before being merged' idea in the TR.
Apr 23 2021
Apr 22 2021
Apr 7 2021
Thanks for the test case. I have verified that it fails as expected without this patch (with D91722 applied).
Apr 6 2021
Sorry for the delay, have been on parental leave. Will look at this later today or tomorrow.
Jan 29 2021
Added code owners for big-endian Sparc, PPC and MIPS as reviewers.
Jan 27 2021
Right, perhaps we should add maintainers of those targets as reviewers since they may be more interested in documenting endianness differences than the little-endian crowd?
Jan 22 2021
Jan 21 2021
That makes sense to me but before acting on it we should probably wait a while to see if the other reviewers have some feedback.
Jan 19 2021
Jan 18 2021
Yes, but that is base ARM and it is only MVE that is incorrect. You can see here that things were inconsistent, which is what this patch is fixing:
Jan 17 2021
We were storing predicate registers, such as a <8 x i1>, in the opposite order to how the rest of llvm expects.
Jan 15 2021
Jan 13 2021
Not terribly important but isn't
Jan 12 2021
The issue was found with attached input bbi-51646.ll If necessary it could be added as a regression test but then the question is where (i.e. should it piggyback on some other file).
This commit seem to cause
$ opt -verify -loop-vectorize bbi-51639.ll -S opt: /repo/elavkje/llvm-project-upstream/llvm/lib/Transforms/Utils/LoopVersioning.cpp:47: llvm::LoopVersioning::LoopVersioning(const llvm::LoopAccessInfo &, ArrayRef<llvm::RuntimePointerCheck>, llvm::Loop *, llvm::LoopInfo *, llvm::DominatorTree *, llv m::ScalarEvolution *): Assertion `L->getExitBlock() && "No single exit block"' failed.
Jan 4 2021
Ugh, there is always some pathological case one forgets to handle. Anyway this LGTM and thanks for fixing!
Dec 14 2020
Dec 1 2020
Addressing the issues with use of stored SCEV after IR had been deleted (see https://reviews.llvm.org/D91711).
Nov 27 2020
Thanks for the clarification. The offending patch has been reverted now (in 808fcfe5944755f0).
Nov 26 2020
Right, there is nothing special with llvm.dbg.value besides that it is a debug intrinsic so it is not allowed to affect optimizations and the fact that such intrinsic uses another Value may not inhibit transformation on that Value. I just wanted to point it out since the situation that predecessors of an instruction get removed without the instruction itself is a bit unusual (but otherwise unimportant for this discussion).
Nov 25 2020
Nov 23 2020
Nov 18 2020
Oct 29 2020
Oct 21 2020
Some nits below but in general looks good to me.
Oct 8 2020
Oct 6 2020
@nikic, @saugustine, thanks for reverting and having a look. I did not receive any notification mail about those build-bot failures AFAICT. I got two but one failure went away on a subsequent run and the other complained about internal compiler error in g++ which seemed unrelated. Anyway I will try to reproduce locally and address the issue as suggested. Thanks again.
Oct 5 2020
Oct 1 2020
Really minor changes since last approved revision but I would prefer if someone gave a renewed blessing.
Tuns out that previous "safety code" caused problems when bootstraping clang. Apparently it is not safe to do getType() on all SCEVs so instead we now store away the type information and compare on the Value instead of the SCEV.
Sep 30 2020
Added safety code to make sure that we are comparing SCEVs of the same Type.
Sep 29 2020
Addressed minor comments by @bjope
Sep 23 2020
Sep 22 2020
Fixed most of the review remarks.
Sep 21 2020
Updated with full context patch.
Sep 15 2020
Sep 14 2020
Change of appraoch. Instead of hooking into deleteDeadPHIs with an AboutToDeleteCallback we do a pre-pass to store a way the llvm.dbg.value of the loop as well as their SCEV expressions. After LSR has done its thing we go through those stored away llvm.dbg.value and if any of them now has an undef variable location we try to update it using its stored SCEV.
Sep 11 2020
Sep 10 2020
Jun 4 2019
What happens if a target decides late (like addPreEmitPass() late) that a hardware loop is no longer possible due to reasons that are not detectable from the IR level hook. For PowerPC this does not seem to be an issue, or at least I cannot find it in the code, but for other targets there could be limitations that you need to analyse the actual machine instructions to find out about. Since the original IV chain has been replaced by intrinsic llvm.loop.decrement.reg (or whatever it iselected to) it seems we now would need to manually insert instructions to update the loop counter which could possibly be a bit hairy to do this late.
Jun 3 2019
May 21 2019
This is interesting. Our (Ericsson's) out-of-tree target has hardware loops and we currently do a similar thing i.e.
May 13 2019
The DW_OP_LLVM_convert stuff looks-good-to-me but I don't feel that I have authority to approve for the rest so please don't wait for any input from my part.
May 9 2019
May 6 2019
May 3 2019
Superseded by https://reviews.llvm.org/D61499
Apr 30 2019
Apr 29 2019
Can (or should) I proceed to push this again? I am a bit confused since the accepted status remains even though I upload new patches.
Apr 26 2019
Have you verified that this results in the desired DW_OPs being emitted in the end and that the (or a) debugger makes sense out of it?
Use MI.isIndirectDebugValue() and improved the test a bit.
Apr 25 2019
After studying the reproducer and reading some in https://llvm.org/docs/SourceLevelDebugging.html I realize that maybe isImplicit() is not a sufficient condition and that we also need to include the second operand of the DBG_VALUE in the test.
Apr 18 2019
! In D59687#1468571, @probinson wrote:
It sounds like you're trying to do this with aggregate types, and that won't work. Only base types (for DWARF 5) or address-like types (for DWARF <= 4) should end up on the stack.
Apr 16 2019
The Chromium compilation segfault comes down to two issues:
Apr 13 2019
Has been reverted in
commit 0366e3e18142466e4dd99d3109d8facd093cedc8 (llvm.org/master, master) Author: Hans Wennborg <firstname.lastname@example.org> Date: Fri Apr 12 12:54:52 2019 +0000