Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Support may_alias in processTypeAttrs() #11273

Open Quuxplusone opened 13 years ago

Quuxplusone commented 13 years ago
Bugzilla Link PR11082
Status NEW
Importance P normal
Reported by Nico Weber (nicolasweber@gmx.de)
Reported on 2011-10-06 19:22:40 -0700
Last modified on 2011-10-07 14:20:37 -0700
Version trunk
Hardware PC All
CC chandlerc@gmail.com, efriedma@quicinc.com, llvm-bugs@lists.llvm.org, rjmccall@apple.com, scshunt@csclub.uwaterloo.ca
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also
clang now complains about unsupported attributes on types. As a consequence, it
complains about may_alias in cast expressions such as here:

void g(int p __attribute__((may_alias))) {}

void f() {
  int a = 0;
  g((int __attribute__((may_alias)))a);
}

hummer:clang thakis$ ../../Release+Asserts/bin/clang -Wall test.c -c
test.c:5:25: error: 'may_alias' attribute ignored when parsing type
  g((int __attribute__((may_alias)))a);
                        ^~~~~~~~~
1 error generated.

This is used in glib, see e.g. here:
http://codesearch.google.com/#cZwlSNS7aEw/external/bluetooth/glib/glib/gatomic.h&q=%5C(g_atomic_pointer_set%5C)%5C%20%5C(%5C(volatile%5C%20gpointer%5C%20G_GNUC_MAY_ALIAS%5C%20%5C*%5C)%5C%20%5C(void%5C%20%5C*%5C)%5C%20%5C(atomic%5C),%5C%20%5C(newval%5C)%5C)%5C)&type=cs&l=76
Quuxplusone commented 13 years ago

It's arguably an improvement over what we did before (i.e. ignore them).

Quuxplusone commented 13 years ago
Going back to *specifically* ignoring this would be an okay short-term hack if
we don't otherwise have this working for 3.0.
Quuxplusone commented 13 years ago

Attributes are by their nature an implementation-specific thing. When using standard C++ attribute syntax, implementations are required to successfully translate programs using attributes with which the implementation is unfamiliar.

I see no reason that this should not apply similarly to non-standard attribute syntax. We should have unknown attributes be at the very least a warning by default, and of course have an appropriate diagnostic group.

Quuxplusone commented 13 years ago

I'm very bothered by this. It regresses essentially all Linux builds w/ Clang. =[ Ignoring this attribute wasn't breaking any of the Linux users, even if its the wrong thing to do here.

I'm looking at a quick fix in the hope that we don't need to revert the original patch, but I think it's really bad to just regress on two platforms two weeks before 3.0's release....

Quuxplusone commented 13 years ago
(In reply to comment #2)
> Going back to *specifically* ignoring this would be an okay short-term hack if
> we don't otherwise have this working for 3.0.

I've done this in r141381 while we sort out what the correct solution is here.
For reference, regardless of the 3.0 release, this regressed trunk too
significantly to leave un-patched for long.

It's really unclear to me what we should do with this attribute on a cast. I
don't like just warning because I agree that these types of attributes *must*
be supported if they are ever used. However, I also don't know what (if any)
semantics to give such a cast in C++...
Quuxplusone commented 13 years ago
(In reply to comment #5)
> It's really unclear to me what we should do with this attribute on a cast. I
> don't like just warning because I agree that these types of attributes *must*
> be supported if they are ever used. However, I also don't know what (if any)
> semantics to give such a cast in C++...

may_alias is semantically a type qualifier: it's something that changes the
semantics of operations on lvalues with the given qualifier, like volatile or
restrict or __attribute((aligned(1))), and it gets stripped off as part of the
lvalue-to-rvalue conversion.  The tricky bit is how exactly pointer conversions
work with this, i.e. do we allow an implicit conversion from int
__attribute((may_alias))* to int* or vice versa.
Quuxplusone commented 13 years ago
Yeah, note that the use of may_alias in Nico's example code is completely
useless;  this is something we would likely want to warn about.

If we followed the usual subtyping rules, we would allow an implicit conversion
from int* to int may_alias*.  This is safe because it makes the optimizers
strictly more cautious.

IR-generation would implement the semantics of may_alias by suppressing the use
of TBAA annotations on operations on such l-values.  This is something that
works in C++ just as well as it does in C.