Previously this would sporadically crash as TargetType was never initialized.
I just mimiced what the basic model does here, marking it as free. I'm happy to change this to Basic if folks think it's more appropriate.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
include/llvm/Analysis/TargetTransformInfoImpl.h | ||
---|---|---|
677 ↗ | (On Diff #113159) | Note, this is not strictly part of the fix (but it's needed to trigger it deterministically, as on my machine the uninitialized pointer get weird values). |
LGTM. Thanks for addressing this issue, this is definitely an improvement and much better than passing an uninitialized value to isLegalAddressingMode.
I think to mirror the behavior of isLegalAddressingMode in this file as closely as possible, we should return TTI::TCC_Free, if !BaseGV, else TTI::TCC_Basic.
The other minor nit is just about the position of the early exit, and I think either position is fine.
include/llvm/Analysis/TargetTransformInfoImpl.h | ||
---|---|---|
706 ↗ | (On Diff #113159) | If I understand correctly, we use this early exit to cover the case where TargetType is NULL, because we didn't execute any iterations of the loop above. I think it would be slightly clearer to just move the early exit before TargetType, with a comment that this covers the case where TargetType would be null. |