[cfe-commits] [PATCH] Potential fix for PR9614

[Rafael Ávila de Espíndola]

> You're right to say that that it isn't a property of the call, at least
> at the C level.  It seems more like a property of the reference to
> "foo" (i.e. whether the reference is via an asm label or not).
> 
> I don't know if we should use aliases to represent this property
> though.  Perhaps a cleaner solution would be to introduce a new
> Constant kind which represents a GlobalValue reference via an asm
> label.  For global variables and undefined and non-available_externally
> functions, this could constant fold to the GlobalValue itself.

I recently found a case with a similar problem that uses builtin
functions instead of asm labels:

inline __attribute__ ((__always_inline__)) char *strrchr(const char
*__s, int __c)  {
  return __builtin_strrchr (__s, __c);
}
char *f(const char *x) { return strrchr(x, '.'); }

Without the extension to the pr9614 fix it would produce

define  i8* @f(i8* %x) nounwind uwtable {
  %1 = call i8* @strrchr(i8* %x, i32 46)
  ret i8* %1
}
define available_externally i8* @strrchr(i8* %__s, i32 %__c) nounwind
uwtable inlinehint alwaysinline {
  %1 = call i8* @strrchr(i8* %__s, i32 %__c) nounwind
  ret i8* %1
}

This got me wondering how much optimization are we missing by not
supporting things like this. I think I agree with Eli and Peter that
this should be handled with an alias like construct, but unlike a real
alias linkage and visibility are meaningless. It also doesn't show up in
codegen and it is handling during linking would be different, so we
should probably have a new construct, lets say external_reference.

The above strrchr could be compiled to

@external_strrchr = external_reference i8* (i8*, i32)* @strrchr
define available_externally i8* @strrchr(i8* %__s, i32 %__c) nounwind
uwtable inlinehint alwaysinline {
  %1 = call i8* @external_strrchr(i8* %__s, i32 %__c) nounwind
  ret i8* %1
}

Now optimizations should work correctly since they see two different
entities, strrchr can be inlined, etc.

external_reference would still be a very special construct:

* It probably only makes sense for it to point to available_externally
functions.
* When the available_externally function is dropped and becomes a
declaration, uses the external_reference should be changed to use the
declaration directly
* If during linking the available_externally function is replaced with a
real definition, the uses of the external_reference should now use the
definition.

> 
> Thanks,

Cheers,
Rafael