[PATCH] Add a dereferencable attribute

Philip Reames listmail at philipreames.com
Thu Jul 10 15:40:50 PDT 2014


On 07/10/2014 03:28 PM, hfinkel at anl.gov wrote:
> As Nick pointed out initially, this actually does make sense. Many 
> targets fool with the actual parameter types for ABI reasons 
> (including "padding" them), and it is important to be able to specify 
> that only part of the type size is dereferenceable. 
Please clarify this in the documentation.

I honestly find this distasteful, but I'll accept it as reality.  :)
>
>> - the combination of a refactoring and a functional change
> There really isn't any (aside from the one renaming, which I can certainly commit separately).
This was the one I was referring to.  Thanks.
>
> ================
> Comment at: lib/AsmParser/LLParser.cpp:1630
> @@ +1629,3 @@
> +  if (!EatIfPresent(lltok::kw_dereferencable))
> +    return false;
> +  LocTy ParenLoc = Lex.getLoc();
> ----------------
> This doesn't seem right.  It seems like this should only be called
> from situations where you already have identified the dereferencable
> keyword.  Should this be an assert?
> This is done in exactly the same way as the other parametrized attributes. Maybe all of them need some work, but that's a separate issue.
I'll accept that answer.  :)
>
> ================
> Comment at: lib/IR/Attributes.cpp:430
> @@ -400,2 +429,3 @@
>     case Attribute::JumpTable:       return 1ULL << 45;
> +  case Attribute::Dereferencable:  return 4095ULL << 46;
>     }
> ----------------
> This encoding seems "unpleasant".  We already except string
> attributes from the masking scheme, would it make sense to separate
> integer attributes as well?  You'd need to leave handling for the
> existing two, but that might be least ugly option.
> I'm not sure, and as I recall, we don't actually accept string attributes on parameters, so some more-general changes would be needed.
Ah, I was unaware of that.  Thanks.
>
>> Also, 4096 is a fairly arbitrary limit.  I could see cases where a
>> array passed by reference would be useful larger than this.  Why not
>> have the units be in units of element size?  i.e. i32*
>> dereferencable(3) would be 12 bytes.  This might make the restricted
>> range much more powerful.  (e.g. [i32 x 20000]* dereferencable(1) is
>> a pointer to a array of 20000 elements all of which are
>> dereferencable.)
> You don't need to convince me to not like the limit, I don't like it. Someone with a better understanding of the bitcode format, I hope, will be able to suggest something better.
Agreed.  :)


Philip



More information about the llvm-commits mailing list