Yes, but the method is local to this file and all the callers already have a type.
Do we still need to pass the DataLyaout to these functions if they have the size & alignment?
Where did this case come from?
Where'd this check go?
Why is it only '1', rather than 'Bytes' used below?
Yes, it's used all over the place in ValueTracking (to get the sizes of other types found during the process).
Previously, if Align was 0, then BaseAlign would take over.
Based on the fact that you can't get the Size necessary to call this function if you have an unsized type.
But I can add the check back, as if (Size > 0), if you want.
I recall this being i8*, but if that is not the case, I'll use Bytes instead.
This is a tradeoff we have to make anyway. E.g. in this revision you introduce more calls to DL.getTypeStoreSize() than you remove. IMHO there is no way to avoid this if we want to make pointers opaque.
Anyway, isSafeToPromoteArgument() has only two callers, one of these is the method itself.
Sorry @dblaikie, but I almost missed these comments as they weren't showing up in Differential.
I've tried to manually quote them here so that the review makes sense.
There are only 4 possible cases, for a GEP getting a T field in S:
The alignof(S) < alignof(T) case seems interesting to me: can an aggregate have a smaller alignment than one of its fields?
And I'm not sure what, if any, bounds are placed on explicit alignments.
So let's say I want to preserve the old behavior.
But we need to discuss which semantic is *correct*:
If Base is aligned to Align and Offset is a multiple of Align (checked below), then the resulting pointer should still be aligned.
For this to have been safe in the old implementation, alignof(S) >= alignof(T) must hold, otherwise an aligned Base might not result in an aligned field.
However, if alignof(S) > alignof(T), and no explicit alignment was provided, the old implementation was overly conservative itself and required that Base be aligned to alignof(S) even though alignof(T) would suffice.
All callers I've encountered would now trip an assert were this case to occur, so at least it wasn't tested against.
Ah, I remember now, the check here is actually that if isDereferenceablePointer returns true, and we have an Argument, the information must come from a dereferenceable attribute.
I suppose it might be better to just skip isDereferenceablePointer and just copy the dereferenceable attribute.
This intrinsic (experimental.gc.relocate) is one of the ones which happen to keep all of their pointee information behind a pointer type and will need to be redesigned (slightly).