User Details
- User Since
- May 19 2020, 6:23 AM (174 w, 4 d)
Thu, Sep 21
Will be continued as a GitHub PR
Tue, Aug 29
Jul 27 2023
Jul 26 2023
Here's a bit more context:
Jul 12 2023
Remove to_string usages
Thank you for reviewing, and yes, I totally aggree. Preventing such issues from happening in the first place is would be better.
Will update this change tomorrow to drop the to_string calls.
Is there anything missing for this to be committed?
I created a fix: https://reviews.llvm.org/D155093
Jul 11 2023
I thought about static_cast'ing Val instead of changing the format string. In case the underlying type of Val changes in the future, one would otherwise have to adapt the format string as well.
Or is there a way to emit a compiler warning if the format string mismatches? You mentioned marking llvm::format with a format attribute, @eli.friedman? Does that trigger a warning?
I think I figured it out:
Jul 10 2023
Here is the IR code of that specific function. The first load has an align of 4, which used to be excluded by the load optimization before this commit.
Jul 9 2023
I managed to reproduce this and isolate the function responsible for the triggering the bug. It's llvm::format_object<long long>::snprint(char*, unsigned int) const. I haven't dug deeper yet, but that's at least some progress :D
Jul 6 2023
@DavidSpickett do you have instructions for me on how to setup everything to reproduce the problem.
So I can support you with the investigations.
Jul 3 2023
Jun 26 2023
Jun 23 2023
as soon as this is commited,I will try to adjust the ARMLoadStoreOptimizer accordingly. @eli.friedman you have commit permissions, right? would you mind committing this change for me with the name and email provided above?
Jun 21 2023
Thank you for the hint. Using update_llc_test_checks.py makes writing the tests much easier.
Jun 20 2023
Added an armv7 strict case to make sure it is identical to non-strict for align 4
I added an align 4 test for armv7 and armv6 with and without strict alignment.
I omitted armv5 here, as, due to the missing strict align, the test generates an ldrd which is probably not something one wants to explicitly test for.
Question is, should I add strict align to armv4/v5 test cases here and adjust the test accordingly?
Jun 19 2023
Thank you for investigating it. I have added a helper called getDualLoadStoreAlignment. Feel free to suggest a better name if you have one.
Jun 15 2023
Jun 13 2023
I did have Align(4) initially, but saw that it was done differently here: https://github.com/llvm/llvm-project/blob/2a1716dec57e8b3dd668df17ecbedfc77a4112e5/llvm/lib/Target/ARM/ARMLoadStoreOptimizer.cpp#L2294
I simply copied the expression. If that is not applicable here, I can just use 4 instead or respect allowsUnalignedMem.
Feb 6 2022
Seems like this can be merged, right?
Jan 17 2022
Jan 3 2022
Dec 31 2021
Comments were addressed
Dec 22 2021
A call to isCompatibleWithMSVC was added with the proper version that introduced the change (1925) and comments describing the intention of the change were added.