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
Event Timeline
include/llvm/Analysis/TargetTransformInfoImpl.h | ||
---|---|---|
677 | 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 | 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. |
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).