- Jan 30, 2022
-
-
Markus Böck authored
Both IDFCalculatorBase and its accompanying DominatorTreeBase only supports pointer nodes. The template argument is the block type itself and any uses of GraphTraits is therefore done via a pointer to the node type. However, the ChildrenGetterTy type of IDFCalculatorBase has a use on just the node type instead of a pointer to the node type. Various parts of the monorepo has worked around this issue by providing specializations of GraphTraits for the node type directly, or not been affected by using specializations instead of the generic case. These are unnecessary however and instead the generic code should be fixed instead. An example from within Tree is eg. A use of IDFCalculatorBase in InstrRefBasedImpl.cpp. It basically instantiates a IDFCalculatorBase<MachineBasicBlock, false> but due to the bug above then goes on to specialize GraphTraits<MachineBasicBlock> although GraphTraits<MachineBasicBlock*> exists (and should be used instead). Similar dead code exists in clang which defines redundant GraphTraits to work around this bug. This patch fixes both the original issue and removes the dead code that was used to work around the issue. Differential Revision: https://reviews.llvm.org/D118386
-
Kazu Hirata authored
Identified with modernize-use-default-member-init.
-
- Jan 24, 2022
-
-
Jeremy Morse authored
As mentioned in discussion of D116821, it's better to just delete this block than keep it hanging around.
-
Jeremy Morse authored
Over in the comments for D116821, some use-cases have cropped up where there's a substantial increase in memory usage. A quick inspection shows that a) it's a lot of memory and b) there are several things to be done to reduce it. Reverting (via disabling this feature by default) to avoid bothering people in the meantime.
-
- Jan 20, 2022
-
-
Nikita Popov authored
Instead of constructing DebugVariables and looking up the order in the comparison function, compute the order upfront and then sort a vector of (order, instr). This improves compile-time by -0.4% geomean on CTMark ReleaseLTO-g. Differential Revision: https://reviews.llvm.org/D117575
-
- Jan 18, 2022
-
-
Nikita Popov authored
-
Nikita Popov authored
Just replacing std::map with DenseMap here is a major regression -- because this code used an identity hash for ValueIDNum. Because ValueIDNum is composed of multiple components, it is important that we use a reasonably good hash function here, so switch it to hash_value. DenseMapInfo::getHashValue<uint64_t> would not be sufficient. This gives a -0.8% geomean improvement on CTMark ReleaseLTO-g.
-
- Jan 13, 2022
-
-
Eugene Zhulenev authored
Reviewed By: cota Differential Revision: https://reviews.llvm.org/D117162
-
- Jan 12, 2022
-
-
Jeremy Morse authored
This feature was previously controlled by a TargetOptions flag, and I figured that codegen::InitTargetOptionsFromCodeGenFlags would default it to "on" for all frontends. Enabling by default was discussed here: https://lists.llvm.org/pipermail/llvm-dev/2021-November/153653.html and originally supposed to happen in 3c045070, but it didn't actually take effect, as it turns out frontends initialize TargetOptions themselves. This patch moves the flag from a TargetOptions flag to a global flag to CodeGen, where it isn't immediately affected by the frontend being used. Hopefully this will actually cause instr-ref to be on by default on x86_64 now! This patch is easily reverted, and chances of turbulence are moderately high. If you need to revert, please consider instead commenting out the 'return true' part of llvm::debuginfoShouldUseDebugInstrRef to turn the feature off, and dropping me an email. Differential Revision: https://reviews.llvm.org/D116821
-
- Jan 02, 2022
-
-
Kazu Hirata authored
Identified by modernize-redundant-void-arg.
-
- Dec 04, 2021
-
-
Kazu Hirata authored
-
- Nov 30, 2021
-
-
Jeremy Morse authored
InstrRefBasedLDV used to crash on the added test -- the exit block is not in scope for the variable being propagated, but is still considered because it contains an assignment. The failure-mode was vlocJoin ignoring assign-only blocks and not updating DIExpressions, but pickVPHILoc would still find a variable location for it. That led to DBG_VALUEs created with the wrong fragment information. Fix this by removing a filter inherited from VarLocBasedLDV: vlocJoin will now consider assign-only blocks and will update their expressions. Differential Revision: https://reviews.llvm.org/D114727
-
Jeremy Morse authored
If we have a variable where its fragments are split into overlapping segments: DBG_VALUE $ax, $noreg, !123, !DIExpression(DW_OP_LLVM_fragment_0, 16) ... DBG_VALUE $eax, $noreg, !123, !DIExpression(DW_OP_LLVM_fragment_0, 32) we should only propagate the most recently assigned fragment out of a block. LiveDebugValues only deals with live-in variable locations, as overlaps within blocks is DbgEntityHistoryCalculators domain. InstrRefBasedLDV has kept the accumulateFragmentMap method from VarLocBasedLDV, we just need it to recognise DBG_INSTR_REFs. Once it's produced a mapping of variable / fragments to the overlapped variable / fragments, VLocTracker uses it to identify when a debug instruction needs to terminate the other parts it overlaps with. The test is updated for some standard "InstrRef picks different registers" variation, and the order of some unrelated DBG_VALUEs changes. Differential Revision: https://reviews.llvm.org/D114603
-
- Nov 29, 2021
-
-
Jeremy Morse authored
InstrRefBasedLDV observes when variable locations are clobbered, scans what values are available in the machine, and re-issues a DBG_VALUE for the variable if it can find another location. Unfortunately, I hadn't joined up the Indirectness flag, so if it did this to an Indirect Value, the indirectness would be dropped. Fix this, and add a test that if we clobber a variable value (on the stack in this case), then the recovered variable location keeps the Indirect flag. Differential Revision: https://reviews.llvm.org/D114378
-
- Nov 25, 2021
-
-
Jeremy Morse authored
In some scenarios, usually involving NRVO, we can issue indirect DBG_VALUEs after SelectionDAG, even in instruction referencing mode (if the variable is an argument). If the corresponding argument value is spilt to the stack, then we have: * Indirection from it being on the stack, * Indirection from it being a dbg.declare or a dbg.addr. However InstrRefBasedLDV only emits one level of indirection. This patch adds the second, by adding an extra DW_OP_deref if necessary. The two tests modified fail otherwise -- they feature some NRVO, and require two levels of indirection to be correct. Differential Revision: https://reviews.llvm.org/D114364
-
Jeremy Morse authored
DBG_INSTR_REF's and DBG_VALUE's can end up in blocks that aren't in the lexical scope of their variable. It's arguable as to what we should do about this, however VarLocBasedLDV permits such variable locations to be propagated, so let's allow it in InstrRefBasedLDV. It's necessary for the modified test to work. Differential Revision: https://reviews.llvm.org/D114578
-
- Nov 24, 2021
-
-
Jeremy Morse authored
Almost all of the time, call instructions don't actually lead to SP being different after they return. An exception is win32's _chkstk, which which implements stack probes. We need to recognise that as modifying SP, so that copies of the value are tracked as distinct vla pointers. This patch adds a target frame-lowering hook to see whether stack probe functions will modify the stack pointer, store that in an internal flag, and if it's true then scan CALL instructions to see whether they're a stack probe. If they are, recognise them as defining a new stack-pointer value. The added test exercises this behaviour: two calls to _chkstk should be considered as producing two different values. Differential Revision: https://reviews.llvm.org/D114443
-
Jeremy Morse authored
Avoid un-necessarily recreating DBG_VALUEs on call instructions. In LiveDebugvalues we choose to ignore any clobbers of SP by call instructions, as they're irrelevant to our model of the machine. We currently do so for tracking register values (MTracker); do the same for tracking variable locations (TTracker). Test modified to endure that a duplicate DBG_VALUE is not created after the call in struction in this test. Differential Revision: https://reviews.llvm.org/D114365
-
- Nov 10, 2021
-
-
Kazu Hirata authored
-
- Nov 07, 2021
-
-
Benjamin Kramer authored
-
- Oct 25, 2021
-
-
Jeremy Morse authored
There are a few STL containers hanging around that can become DenseMaps, SmallVectors and similar. This recovers a modest amount of compile time performance. While I'm here, adjust the bit layout of ValueIDNum: this was always supposed to act like a value type, however it seems that clang doesn't compile the comparison functions to act that way. Add a uint64_t to a union that explicitly aliases the bitfields, so that we can compare the whole value as a single integer. Differential Revision: https://reviews.llvm.org/D112333
-
Jeremy Morse authored
This patch is like D111627 -- instead of calculating IDF for every location on the stack, only do it for the smallest units of interference, and copy the PHIs for those units to any aliases. The test added runs placeMLocPHIs directly, and tests that: * A def of the lower 8 bits of a stack slot causes all aliasing regs to have PHIs placed, * It doesn't cause the equivalent location to x86's $ah, which isn't aliased, to have a PHI placed. Differential Revision: https://reviews.llvm.org/D112324
-
Jeremy Morse authored
During register allocation, some instructions can have stack spills fused into them. It means that when vregs are allocated on the stack we can convert: SETCCr %0 DBG_VALUE %0 to SETCCm %stack.0 DBG_VALUE %stack.0 Unfortunately instruction referencing finds this harder: a store to the stack doesn't have a specific operand number, therefore we don't substitute the old operand for a new operand, and the location is dropped. This patch implements a solution: just recognise the memory operand attached to an instruction with a Special Number (TM), and record a substitution between the old value and the new one. This patch adds substitution code to InlineSpiller to record such fused spills, and tracking in InstrRefBasedLDV to recognise such values, and produce the value numbers for them. Everything to do with the movement of stack-defined values is already handled in InstrRefBasedLDV. Differential Revision: https://reviews.llvm.org/D111317
-
Jeremy Morse authored
This patch swaps two lines -- the CurSucc reference can be invalidated by the call to DFS.push_back, therefore that should happen last. The usual hat-tip to asan for catching this. This patch also swaps an ealier call to ToAdd.insert and DFS.push_back, where a stable iterator (from successors()) is being used. This isn't strictly necessary, but is good for consistency and avoiding readers asking themselves why the two code portions have a different order.
-
- Oct 24, 2021
-
-
Kazu Hirata authored
We can erase an item in a set or map without checking its membership first.
-
- Oct 22, 2021
-
-
Jeremy Morse authored
Sometimes we generate code that writes to a subregister, then spills / restores a super-register to the stack, for example: $eax = MOV32ri 0 MOV64mr $rsp, 1, $noreg, 16, $noreg, $rax $rcx = MOV64rm $rsp, 1, $noreg, 8, $noreg This patch takes a different approach: it adds another index to MLocTracker that identifies a size/offset within a stack slot. A location on the stack is then a pari of {FrameIndex, SlotNum}. Spilling and restoring now involves pairing up the src/dest register numbers, and the dest/src stack position to be transferred to/from. Location coverage improves as a result, compile-time performance decreases, alas. One limitation is that if a PHI occurs inside a stack slot: DBG_PHI %stack.0, 1 We don't know how large the resulting value is, and so might have difficulty picking which value to use. DBG_PHI might need to be augmented in the future with such a size. Unit tests added ensure that spills and restores correctly transfer to positions in the Location => Value map, and that different register classes written to the stack will correctly clobber all other positions in the stack slot. Differential Revision: https://reviews.llvm.org/D112133
-
Jeremy Morse authored
This patch adds some unit tests for the machine-location transfer-function building parts of InstrRefBasedLDV: i.e., test that if we feed some MIR into the transfer-function building code, does it create the correct transfer function. There are a number of minor defects that get corrected in the process: * The unit test was selecting the x86 (i.e. 32 bit) backend rather than x86_64's 64 bit backend, * COPY instructions weren't actually having their subregister values correctly represented in the transfer function. Subregisters were being defined by the COPY, rather than taking the value in the source register. * SP aliases were at risk of being clobbered, if an SP subregister was clobbered. Differential Revision: https://reviews.llvm.org/D112006
-
- Oct 20, 2021
-
-
Jeremy Morse authored
Here's another performance patch for InstrRefBasedLDV: rather than processing all variable values in a scope at a time, instead, process one variable at a time. The benefits are twofold: * It's easier to reason about one variable at a time in your mind, * It improves performance, apparently from increased locality. The downside is that the value-propagation code gets indented one level further, plus there's some churn in the unit tests. Differential Revision: https://reviews.llvm.org/D111799
-
- Oct 19, 2021
-
-
Jeremy Morse authored
This is purely a performance patch: InstrRefBasedLDV used to use three DenseMaps to store variable values, two for long term storage and one as a working set. This patch eliminates the working set, and updates the long term storage in place, thus avoiding two DenseMap comparisons and two DenseMap assignments, which can be expensive. Differential Revision: https://reviews.llvm.org/D111716
-
Jeremy Morse authored
This field gets assigned when the relevant object starts being used; but it remains uninitialized beforehand. This risks introducing hard-to-detect bugs if something changes, so zero-initialize the field.
-
- Oct 18, 2021
-
-
Jeremy Morse authored
gcc11 warns that this counter causes a signed/unsigned comaprison when it's later compared with a SmallVector::difference_type. gcc appears to be correct, clang does not warn one way or the other.
-
- Oct 14, 2021
-
-
Jeremy Morse authored
This patch is very similar to D110173 / a3936a6c, but for variable values rather than machine values. This is for the second instr-ref problem, calculating the correct variable value on entry to each block. The previous lattice based implementation was broken; we now use LLVMs existing PHI placement utilities to work out where values need to merge, then eliminate un-necessary ones through value propagation. Most of the deletions here happen in vlocJoin: it was trying to pick a location for PHIs to happen in, badly, leading to an infinite loop in the MIR test added, where it would repeatedly switch between register locations. The new approach is simpler: either PHIs can be eliminated, or they can't, and the location of the value is a different problem. Various bits and pieces move to the header so that they can be tested in the unit tests. The DbgValue class grows a "VPHI" kind to represent variable value PHIS that haven't been eliminated yet. Differential Revision: https://reviews.llvm.org/D110630
-
Jeremy Morse authored
Some functions get opted out of instruction referencing if they're being compiled with no optimisations, however the LiveDebugValues pass picks one implementation and then sticks with it through the rest of compilation. This leads to a segfault if we encounter a function that doesn't use instr-ref (because it's optnone, for example), but we've already decided to use InstrRefBasedLDV which expects to be passed a DomTree. Solution: keep both implementations around in the pass, and pick whichever one is appropriate to the current function.
-
- Oct 13, 2021
-
-
Jeremy Morse authored
In D110173 we start using the existing LLVM IDF calculator to place PHIs as we reconstruct an SSA form of machine-code program. Sadly that's slower than the old (but broken) way, this patch attempts to recover some of that performance. The key observation: every time we def a register, we also have to def it's register units. If we def'd $rax, in the current implementation we independently calculate PHI locations for {al, ah, ax, eax, hax, rax}, and they will all have the same PHI positions. Instead of doing that, we can calculate the PHI positions for {al, ah} and place PHIs for any aliasing registers in the same positions. Any def of a super-register has to def the unit, and vice versa, so this is sound. It cuts down the SSA placement we need to do significantly. This doesn't work for stack slots, or registers we only ever read, so place PHIs normally for those. LiveDebugValues choses to ignore writes to SP at calls, and now have to ignore writes to SP register units too. Differential Revision: https://reviews.llvm.org/D111627
-
Jeremy Morse authored
Old versions of gcc want template specialisations to happen within the namespace where the template lives; this is still present in gcc 5.1, which we officially support, so it has to be worked around.
-
Jeremy Morse authored
InstrRefBasedLDV used to try and determine which values are in which registers using a lattice approach; however this is hard to understand, and broken in various ways. This patch replaces that approach with a standard SSA approach using existing LLVM utilities. PHIs are placed at dominance frontiers; value propagation then eliminates un-necessary PHIs. This patch also adds a bunch of unit tests that should cover many of the weirder forms of control flow. Differential Revision: https://reviews.llvm.org/D110173
-
- Oct 12, 2021
-
-
Jeremy Morse authored
These "dump" methods call into MachineOperand::dump, which doesn't exist with NDEBUG, thus we croak. Disable LiveDebugValues dump methods when NDEBUG is turned on to avoid this.
-
Jeremy Morse authored
This patch shifts the InstrRefBasedLDV class declaration to a header. Partially because it's already massive, but mostly so that I can start writing some unit tests for it. This patch also adds the boilerplate for said unit tests. Differential Revision: https://reviews.llvm.org/D110165
-
- Oct 07, 2021
-
-
Jack Andersen authored
Based on the reasoning of D53903, register operands of DBG_VALUE are invariably treated as RegState::Debug operands. This change enforces this invariant as part of MachineInstr::addOperand so that all passes emit this flag consistently. RegState::Debug is inconsistently set on DBG_VALUE registers throughout LLVM. This runs the risk of a filtering iterator like MachineRegisterInfo::reg_nodbg_iterator to process these operands erroneously when not parsed from MIR sources. This issue was observed in the development of the llvm-mos fork which adds a backend that relies on physical register operands much more than existing targets. Physical RegUnit 0 has the same numeric encoding as $noreg (indicating an undef for DBG_VALUE). Allowing debug operands into the machine scheduler correlates $noreg with RegUnit 0 (i.e. a collision of register numbers with different zero semantics). Eventually, this causes an assert where DBG_VALUE instructions are prohibited from participating in live register ranges. Reviewed By: MatzeB, StephenTozer Differential Revision: https://reviews.llvm.org/D110105
-
- Oct 05, 2021
-
-
Jeremy Morse authored
An important part of the instruction referencing solution is that we identify all the registers that values move between before we then compute an SSA-like function from the machine code, and from the variable intrinsics. DBG_PHIs weren't causing all the subregisters of their operands to be tracked; this patch forces that to happen. The practical implications were that not enough space is allocated for storing values when analysing the function -- asan will crash on the attached test case with an unpatched compiler. Non-asan llc's will produce a DBG_VALUE $noreg, where it should be $dil. Differential Revision: https://reviews.llvm.org/D109064
-