Not actively working on LLVM at the moment
User Details
- User Since
- Aug 13 2013, 3:10 PM (464 w, 14 h)
Tue, Jun 28
Hey, I don't know if this has anything to do with this change.
Judging from reaction from folks to older diffs, I have a feeling the script may also be running within apple internal builds...
It's not clear from description that it's done to fix those scripts.
Added test for new llvm-link option.
For background: We had failing builds because the host clang was configured for a default triple of x86_64-redhat-linux-gnu but the symbolizer build using x86_64-unknown-linux-gnu. I suspect this started failing in combination with the new LLVM_ENABLE_PER_TARGET_RUNTIME_DIR default in LLVM expecting libcxx headers and libraries in a subdirectory matching the target triple.
Mon, Jun 27
For background: My understanding of IMPLICIT_DEF is that it was a bit of a workaround/hack necessary because for a given VReg on a control flow join you can have different "undef"/"not undef" merge. Unfortunately being out of SSA form we can't just attach an undef flag to the respective PHI operand as there is no PHI anymore. I always felt a bit uneasy with IMPLICIT_DEF getting used in other situations where they are not at the end of a block just for joining with a different definition/live value in the next block...
I am not sure I understand this change... Wasn't the intention of the existing that code that when you have something like:
%2 = COPY undef %1 USE %2
that you merge the regs to remove the copy and add an undef to the use operands? =>
USE undef %1
Yes, this could potentially regress rematerializing instructions from target load intrinsics. We would have to pass AA through getTgtMemIntrinsic to allow targets to set the flag themselves.
Maybe we should leave this diff open for a day or two to wait for extra feedback?
I always found it very unfortunate that we keep IR references around in MIR to do alias analysis queries. This appears to remove a lot of those users (all but the ones in the schedule graph construction?), so I highly welcome the change!
Thu, Jun 23
Are you sure your bot touble actually comes from GlobalIsel? While GIsel is unfinished and likely broken on x86 it also shouldn‘t be enabled anywhere except a few dedicated globalisel tests…
When there is a bug producing invalid IR then we better crash than hide it.
Mon, Jun 20
LGTM
Could you instead exclude the test in the CMakeLists.txt file? Given that this is source from the gcc torture suite that we may want to update again in the future, it's probably best if we can avoid custom changes in the source.
LGTM
LGTM
LGTM
Thu, Jun 16
LGTM
I didn't go into any detailed review here, but some top of the head thoughs:
- We already have LiveIntervals which was supposed to be good enough for everyone. I agree that there is no efficient API for querying every vreg live at a certain program point. But I'd feel more comfortable adding a convenience or caching layer for that on top of LiveIntervals...
- We already have a 2nd system LiveVariables (which we failed to actually deprecate enough to remove).
- This is adding a 3rd one, so I think it ought to have some better justification than "simplicity over speed".
Wed, Jun 15
Yes that's what I wrote. It seems there is an exception that if for all operands you get readReg() == false (that includes undef) then it is indeed allowed to have a VReg without a definition.
I see this code in the MachineVerifier:
I'm also not sure right now if vregs without definitions are even legal MIR while we are still in MachineSSA... Checking that now
Please choose a better title!
Fri, Jun 10
@JakeEgan I did a quick look for the code in that stacktrace but nothing jumped out on me. It‘s with inlining effects obscuring it. But I think that crash must be investigated on an AIX machine, there‘s no indication that this directly related to my change which works on all other systems.
Mon, Jun 6
LGTM, thanks
oh looks like we also mark the function parameters as live-in causing a bunch of test changes. Anyway I guess this change better reflects the reality of what registers are in the live-in lists. LGTM
The background for this property is that we only really compute the livein lists at the end of register allocation (in the VirtRegRewriter or FastRegAlloc). Before that point using the liveins is pretty much always wrong because they are not complete/correct.
Jun 2 2022
Jun 1 2022
address review feedback. I assume this is accepted and good to push when the buildkit builds look reasonable.
May 31 2022
There's a lot going on here.
- Could you extract the ShouldAllocClass fixes for fastregalloc into a separate diff so we can get discuss them separately and get feedback from AMDGPU folks who introduced this and are the major other user of this AFAIK.
- I am still wrapping my head around tile registers and tile register configs; specifically I wonder if the support for that really needs to be integrated into the generic register allocation code or whether there is a way to materialize the register configurations in a post-pass. For example do you know how the legacy x86 x87-FPU support works, where we use the register allocator to allocate pseudo FP register fp0-fp7 and then use a post-pass in X86FloatingPoint to insert the necessary stack management operations after the fact. I am not saying it's the same problem, but it is an example of an instance where we managed to have the regalloc allocate to some intermediate pseudo registers and adapt to the complications (in that case register stacks) in a target-specific pass so we wouldn't need to introduce the concept of register stacks to the generic code.
May 26 2022
Address review feedback.
llvm/docs/GettingStarted.rst only asks for python >= 3.6 at the moment.
This change is breaking the build for me: BooleanOptionalAction is only available in python 3.9!
May 25 2022
This seems to make sense to me (though I am not super familiar with the code here). Some ideas for clearer code below:
Well I don't have too much experience with STATEPOINTs, but the change makes sense to me so far.
May 24 2022
Enable LTO by default and fix a whole bunch of LTO tests because of it.
LGTM, thanks!
May 23 2022
LGTM but let's wait for @sameerds for accept.
May 19 2022
Changed the approach a bit; summary (and tests) are updated.
While trying to implement things I noticed that -Xclang options are only meant for the LLVM codegeneration part (aka CC1) of clang, they are not meant for the linker/LTO (which would be -Xlinker or -Wl) and consequently there is no easy way in the code to query -Xclang options and pass them on to the linker (nor would it be a good idea I think).
May 18 2022
As far as FrameSetup/FrameDestroy are concerned, it's a bug if the targets break on them.
+1 to adding something to lto::Config.
update tests
Therefore, if reserved registers information is changed between last time freezeReservedRegs is called and RAGreedy, it's not picked up by RCI.
Nice catch! LGTM.
May 17 2022
I am not completely sure however how to deal with users specifying -Xclang -no-opaque-pointers explicitly. With this change they would end up using opaque-pointers anyway in LTO modes...