User Details
- User Since
- Jan 25 2017, 7:29 AM (320 w, 5 d)
Today
I note from an earlier comment that updating VS2022 with latest patches solved this issue for @srj
But is the issue similar for the minimum required version of MSVC (currently Visual Studio 2019 / v16.0 )??It passed across all windows buildbots, which includes VS 2019. Example: https://lab.llvm.org/buildbot/#/builders/13/builds/33254
Feb 13 2023
Feb 10 2023
Fix up ASSERT_TRUE to pass messages correctly
Thanks for the speedy review Jannik
Changes made
Got it. But it is still not good to modify the upstream due the requirement from the downstream. (I know this happens in llvm occasionally. But it is still not good, right?)
I think it may better for you to contribute the other materialization options you mentioned. Or you can try to add a unittest if you don't want to contribute the downstream extension to public for any reason . Otherwise, even all people here get in consensus. It is possible that other developers found the coding here is redundant and optimize it with a NFC patch directly. (This happens a lot in llvm)
Add unit test demonstrating use of extra rematerialization callback
Feb 9 2023
Address review commit (sorry Jannik - missed it before)
Rebase on fixed version
clang-format change
Removed assert that was incorrect (and causing build-bot pre-checkin failures)
Feb 7 2023
Rebased
More changes based on feedback
Feb 6 2023
Thanks for the feedback - see the comments and the udpated patch(es)
Addressing reviewer comments
Addressing reviewer comment
Jan 26 2023
LGTM
Jan 10 2023
Well, this LGTM, but probably need some feedback from others.
Nov 30 2022
Nov 28 2022
Nov 24 2022
Managed to get arc diff to work!
Nov 23 2022
Updated - adding test for builtin
Nov 21 2022
Address review comments
Nov 17 2022
Updating based on review comments
Nov 4 2022
This fixes the issue I was observing. Thanks.
Oct 18 2022
Thanks (sorry I missed it on the original comments)
Is something required for system-libs.windows.test too?
I think that the reason we don't see the same failure for e.g. OpenACC is that there's a unittest/Frontend that requires OpenACC - if I comment out that subdir in the unittests/CMakeLists.txt it fails llvm-config test in the same way for OpenACC. Similarly, if I add FrontendHLSL as an LLVM_LINK_COMPONENT in unittests/Frontend/CMakeLists.txt (even though it isn't), the static lib is generated and llvm-lit llvm-config test passes.
Oct 5 2022
Include address space AS in the pointerto call
Updated for review requests
Thanks - removed the test and made the changes.
Aug 11 2022
Aug 9 2022
Jul 26 2022
LGTM
Jul 25 2022
Jul 15 2022
Added a test
Jul 13 2022
Updated with review suggestions
I wasn't sure whether Mesa would also have issues - I was going to follow that up, so thanks for the info.
I'll update as you suggest to !IsShader
Jul 12 2022
Additionally, not everything marked as inreg actually contributes to the NumUserSGPRs - so the knowledge about which arguments are and aren't would need to be added to the backend.
Jun 16 2022
Adding check lines back into mad_u64_u32.ll
Updated tests to include 1030 specific run lines
Jun 15 2022
Add test updates
Forgot to update the tests. Stand by.
Jun 13 2022
Well spotted. The problem is that it's difficult or impossible to persuade globalisel to use the svs addressing mode in the first place, because it assumes that frame offsets are divergent so doesn't put them in an sgpr for us. See AMDGPURegisterBankInfo.cpp:
I note that this includes global isel changes - but none of the tests are testing it?
Jun 7 2022
This looks reasonable to me, but I think we need someone else to confirm too.
May 19 2022
LGTM
Mar 29 2022
Mar 15 2022
Pushed an earlier commit by mistake
Picked this issue up in some testing - seems a logical fix.
I have a reproducer, but even the cut-down isn't that clean - do you think this requires a test?
Dec 2 2021
Nov 30 2021
Spelling mistake (and rebase for test changes)
Updating test in light of review suggestions
Updating with link to change for which this is a pre-commit
Pre-commit test