Both LowerConstantIntrinsics and InjectTLIMappings fail to preserve the analysis result of GlobalsAA. Not preserving the analysis might affect benchmark performance. This change fixes this issue.
Details
Diff Detail
Event Timeline
| llvm/lib/Transforms/Utils/InjectTLIMappings.cpp | ||
|---|---|---|
| 148 | Thank you for your patch. Didn't you have to add this pass to the tests checking the pass sequence? Have you run make check-all on your changes? Francesco | |
LGTM. Neither InjectTLIMappings nor LowerConstantIntrinsics should modify globals and can preserve GlobalsAA. This is also in line with what we currently do in many other passes.
| llvm/lib/Transforms/Scalar/LowerConstantIntrinsics.cpp | ||
|---|---|---|
| 141 | hmm, there's probably potential for preserving more here. But that's for potential follow-ups. | |
| llvm/lib/Transforms/Utils/InjectTLIMappings.cpp | ||
| 148 | AFAIK if GlobalsAA is only used if it is available. So if it is not preserved, it won't be re-built, so this change won't impact any of the pipeline tests. | |
I can definitely split this into two different patches
| llvm/lib/Transforms/Utils/InjectTLIMappings.cpp | ||
|---|---|---|
| 148 | Thanks for the explanation @fhahn, I did run make check-all as well, and I didnt notice any issues anyway @fpetrogalli | |
| llvm/lib/Transforms/Utils/InjectTLIMappings.cpp | ||
|---|---|---|
| 148 | Thank you for confirming, LGTM. Francesco | |
I've split the changes up:
https://reviews.llvm.org/D82342
https://reviews.llvm.org/D82343
Abandoning as I have split in the following:
https://reviews.llvm.org/D82342
https://reviews.llvm.org/D82343
clang-format: please reformat the code