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.

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to