- User Since
- Dec 17 2012, 10:03 AM (444 w, 8 h)
Thu, Jun 17
Wed, Jun 16
Tue, Jun 8
Fri, Jun 4
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?
Thu, Jun 3
Tue, May 25
LLVM ERROR: unable to translate in big endian mode (in function: f)
Mon, May 24
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?
Oct 26 2020
Oct 21 2020
Thanks all for your help and @nikic in particular for doing the perf measurements and pushing for a better solution.
I think this shows that reviews work and produce better code quality!
- Use Known directly instead of using an intermediate AddrKnownBits variable
- Change AccCstIndices into AccConstIndices
- Move initialization code of IndexBits next to first use
- Regenerate test case using update_test_check
Oct 20 2020
Oups, tagged Eli's old account.
Tagging the new one @efriedma
Did 1 & 2 of @eli.friedman's suggestions.
- Skip zero indices
- Accumulate constant indices with constant scaling factor in a separate variable to avoid calls to computeForAddSub
Thanks @efriedma for the suggestions!
If we remove that from the equation, I expect the compile-time impact of this patch would be pretty minimal. I started some work on this, but I'm not sure when I'll be able to finish it.
Looks reasonable but I'd like to have an idea of the compile time impact of this patch.
Oct 19 2020
Following the conversation in D87342 , this patch has been rebased and all the previous feedbacks have been addressed.
Get rid of TrailZ and track everything with AddrKnownBits.
Oct 16 2020
Add the reviewers from D87342
Oct 9 2020
The general direction is fine, but IMHO there are needless code churn.
Why can't we stick to getCostPerUse instead of changing all the users?
Oct 8 2020
Thanks for the quick review @aemerson!
Oct 7 2020
Could use a test, but it generally makes sense to have such a method.
- Add a test for the new method
Thanks guys for the quick review!
- Move method earlier in the header
- Fix comment in test
- Add getMinValue/getMaxValue
Oct 6 2020
Abandoning this diff in favor of D86364 based on our discussion.