[patch] Clarify and verify what llvm.used is

[Duncan Sands]

Hi Rafael,

On 22/04/13 16:43, Rafael Espíndola wrote:
>>> Yes, basically replace dyn_cast with cast in the users. I was going to
>>> do it in a followup patch, but there are not that many users, so I
>>> included it in the attached one.
>>
>>
>> You forgot to have the verifier complain if the type is not an array type!
>
> There is already a check for only arrays having appending linkage. I
> added a test to make sure it stays that way.

I see, only arrays can have appending linkage, so it's checked elsewhere.

>
>>> Not sure. That would require every user to check for
>>> ConstantAggregateZero.
>>
>>
>> Don't they just loop over the array index range and do stuff per index, in
>> which
>> case this wouldn't be a problem (empty loop)?
>
> Unfortunately no,  ConstantAggregateZero inherits from Constant, not
> ConstantArray, so they would need a dyn_cast.

Not if they get the array length from the array type.  However I'm not going to
press the issue since the minuscule advantage obtained by allowing null doesn't
seem worth it.

>
>>> +  if (GV.hasName() && (GV.getName() == "llvm.used")) {
>>> +    Assert1(!GV.hasInitializer() || GV.hasAppendingLinkage(),
>>> +            "invalid linkage for intrinsic global variable", &GV);
>>> +    Type *GVType = cast<PointerType>(GV.getType())->getElementType();
>>
>>
>> The cast shouldn't be needed as if GV is a GlobalValue then getType is
>> tweaked
>> to return PointerType.
>
> Updated, thanks.
>
> New patch is attached.

LGTM.

Ciao, Duncan.