[llvm-commits] [llvm] r47220 - in /llvm/trunk: lib/Transforms/IPO/GlobalOpt.cpp test/Transforms/GlobalOpt/2008-02-16-NestAttr.ll

[Duncan Sands]

Hi Chris,

> > Remove any 'nest' parameter attributes if the function
> > is not passed as an argument to a trampoline intrinsic.
> 
> Nice.  Out of curiousity, how does nest do to codegen?

'nest' causes a specific register to be grabbed for the
parameter in calls.  So removing it doesn't do much :)

> > +static const ParamAttrsList *StripNest(const ParamAttrsList *Attrs) {
> > +  if (Attrs) {
> > +    for (unsigned i = 0, e = Attrs->size(); i != e; ++i) {
> > +      uint16_t A = Attrs->getParamAttrsAtIndex(i);
> > +      if (A & ParamAttr::Nest) {
> 
> How about:
>    if ((A & ParamAttr::Nest) == 0) continue;
> 
> To avoid indentation in the loop.

OK, if you like.

> Alternatively, maybe ParamAttrsList   
> should have a 'find attribute' function that returns the first  
> argument that has the specified attribute?

I think that would only be useful for attributes that can occur at
most once, i.e. sret and nest, and since sret is always on the first
parameter it's not useful for sret.  I did consider a
removeAttrEverywhere method, but again it didn't seem generally
useful.
 
> > +static void RemoveNestAttribute(Function *F) {
> > +  F->setParamAttrs(StripNest(F->getParamAttrs()));
> > +  for (Value::use_iterator UI = F->use_begin(), E = F->use_end();  
> > UI != E;++UI){
> > +    Instruction *User = cast<Instruction>(*UI);
> 
> > +    if (CallInst *CI = dyn_cast<CallInst>(User)) {
> 
> Please use CallSite to handle CallInst/Invoke uniformly.

To create a CallSite don't you first need to get your hands on either a CI
or an II?  Which means doing the dyn_cast and test anyway, to produce the
CallSite (this is annoying, maybe it should be possible to create one from
an Instruction*).  So I don't see that it gains you much.  Also, in
ChangeCalleesToFastCall a few lines above I see that a certain "lattner"
didn't consider a CallSite useful for the calling convention case :)

Ciao,

Duncan.