- User Since
- Dec 17 2012, 10:03 AM (468 w, 2 d)
Tue, Nov 30
LGTM, nits below
Mon, Nov 29
Tue, Nov 16
Mon, Nov 15
should we go ahead and revert (most of*) https://reviews.llvm.org/D35816?
Nov 2 2021
I'm also guessing that you'd want to avoid using string keys for the function-attribute implementation?
Let me just step back a little bit and say that now that I think about what we did, having something that answers "should I run in this instance" is desirable, the implementation doesn't really matter. We did it with function attributes, but having a bisect client API like you're introducing is fine.
My only complain is that the client interface should not have remote in the name :P.
Oct 27 2021
Oct 26 2021
Oct 1 2021
I don't know if it's safe to rely on the kill flag being on the first operand that uses a particular reg. If not I'll rework the patch.
This seems accidental. I think ideally we would enforce consistency of the flag within an instruction. Short of that we better not assume anything like just the first operand having a kill flag...
Sep 27 2021
Makes sense, thanks!
Sep 24 2021
This mostly seems to benefit X86
Sep 17 2021
One last comment, otherwise LGTM.
Sep 16 2021
Sep 14 2021
Aug 31 2021
Aug 12 2021
Looks good to me as well.
Jul 28 2021
Given that, can you approve Philip's patch?
Following the conversation in https://reviews.llvm.org/D105723#2894209, approving this patch.
Jul 21 2021
Jul 13 2021
I've tried approaches with target pseudo intrinsics and extra IR passes, but still everything depends on missing the external (relatively to the IRTranslator) way to obtain the info about Value->Vreg mapping.
I am not sure I understand the issue here.
Could you paste the IR with the wrong transformation (before this patch) for one of the test (e.g., the smaller one)?
Jul 7 2021
Jun 29 2021
I'm suspicious of how simple this patch is, and not sure why this special phi handling is needed given that refineSubRanges was already called
Jun 17 2021
Jun 16 2021
Jun 8 2021
Jun 4 2021
By removing the reportTranslationError() temporarily in order to test my implementation, that would cause errors in other targets. Can you please suggest me any other way to solve this issue?
Jun 3 2021
May 25 2021
LLVM ERROR: unable to translate in big endian mode (in function: f)
May 24 2021
Add GlobalISel infrastructure up to the point where we can select a ret void.
May 19 2021
I wouldn't be so sure about that. One particular usecase I need this for is a bitcast between pointer types:
%1 = bitcast i8* %0 to %struct.ST*
In our target they're both of p0 LLTs, so the default method eliminates this instruction while in our target it's necessary to keep it.
May 12 2021
Disclaimer: I didn't really look at the AMDGPU changes.
May 3 2021
I was musing some more and it occurred that some values can come from this block
Apr 29 2021
Apr 27 2021
Could you add dedicated test case?
Thanks for fixing this.
Mar 31 2021
Mar 23 2021
Mar 22 2021
Could you add a test case exposing the problem?
Mar 16 2021
I agree with @mtrofin's assessment, the previous code was incorrect and if we have a compile time hit, at least short term, I think this is the right thing to do.
Mar 10 2021
Thanks for the follow-up patch!
Mar 5 2021
For targets that make use of HW modes, register sizes are specific to a subtarget.
Mar 4 2021
Feb 26 2021
Feb 25 2021
I think this is actually a problem GlobalISel does have since it doesn't have the DAG scheduler/linearizer to minimize physical register live ranges. We need an MI equivalent pass to do this
Feb 18 2021
It sounds like we need to have the fast-RA be able to defer to the more sophisticated allocators in certain cases.
Feb 10 2021
To elaborate on the answer:
why is it any more job of the scheduler to keep the live ranges small for the sake of the allocator?
I'm not an expert in this area, but why is it any more job of the scheduler to keep the live ranges small for the sake of the allocator? Shouldn't the allocator be able to deal with any valid input (within computational reason)?
Feb 8 2021
Looks good, thanks fixing the test!
the list scheduler has register pressure tracking disabled I think.
How was this test working before?
Feb 4 2021
Is it something we could fix in the scheduler instead?
Feb 3 2021
LGTM with nit on the function name.
Jan 28 2021
LGTM, just make sure to move getRegisterCostTableIndex under protected.
Jan 26 2021
Jan 21 2021
I'll try to take a look by end of next week.
Jan 11 2021
Jan 7 2021
Dec 18 2020
Dec 15 2020
Dec 3 2020
Nov 6 2020
Nov 4 2020
Oct 30 2020
This is allowed by the current definition. See D85603
Oct 27 2020
I guess the compiling time increase on these benchmarks should be acceptable?