This revision was automatically updated to reflect the committed changes.
Closed by commit rL334174: [CodeGen] Improve diagnostics related to target
attributes (authored by GBuella, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D46541
craig.topper accepted this revision.
craig.topper added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D46541
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/li
GBuella marked an inline comment as done.
GBuella added inline comments.
Comment at: test/CodeGen/target-features-error-2.c:39
__m128d need_avx(__m128d a, __m128d b) {
return _mm_cmp_sd(a, b, 0); // expected-error {{'__builtin_ia32_cmpsd' needs
target feature avx}}
}
--
GBuella updated this revision to Diff 150018.
https://reviews.llvm.org/D46541
Files:
lib/CodeGen/CodeGenFunction.cpp
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/CodeGenModule.h
test/CodeGen/target-features-error-2.c
test/CodeGen/target-features-error.c
Index: test/CodeGen/target-features
craig.topper added inline comments.
Comment at: test/CodeGen/target-features-error-2.c:39
__m128d need_avx(__m128d a, __m128d b) {
return _mm_cmp_sd(a, b, 0); // expected-error {{'__builtin_ia32_cmpsd' needs
target feature avx}}
}
GBuella wrote:
> craig.top
GBuella added inline comments.
Comment at: test/CodeGen/target-features-error-2.c:39
__m128d need_avx(__m128d a, __m128d b) {
return _mm_cmp_sd(a, b, 0); // expected-error {{'__builtin_ia32_cmpsd' needs
target feature avx}}
}
craig.topper wrote:
> The 4 com
craig.topper added inline comments.
Comment at: test/CodeGen/target-features-error-2.c:39
__m128d need_avx(__m128d a, __m128d b) {
return _mm_cmp_sd(a, b, 0); // expected-error {{'__builtin_ia32_cmpsd' needs
target feature avx}}
}
The 4 compare functions he
GBuella added a comment.
In https://reviews.llvm.org/D46541#1106743, @craig.topper wrote:
> I think you can pass StringRef(F).substr(1). That won't create a temporary
> string. It will just create a StringRef pointing into the middle of an
> existing std::string stored in the parsed attributes.
GBuella updated this revision to Diff 147984.
https://reviews.llvm.org/D46541
Files:
lib/CodeGen/CodeGenFunction.cpp
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/CodeGenModule.h
test/CodeGen/target-features-error-2.c
test/CodeGen/target-features-error.c
Index: test/CodeGen/target-features
craig.topper added a comment.
I think you can pass StringRef(F).substr(1). That won't create a temporary
string. It will just create a StringRef pointing into the middle of an existing
std::string stored in the parsed attributes.
https://reviews.llvm.org/D46541
_
GBuella updated this revision to Diff 147775.
GBuella added a comment.
Fixed a horrible bug in the patch. Adding a ref to temporary string is not a
wise thing to do, so I had to remove this line:
ReqFeatures.push_back(F.substr(1));
Now the ReqFeatures vector can also refer to strings starting
echristo added a comment.
I think this will work, one inline comment. Might also be good to get a few
different test cases, e.g. one where we're not seeing the alphabetically first
as the minimum :)
Comment at: lib/CodeGen/CodeGenModule.h:1085
+ TargetAttr::ParsedTargetAtt
craig.topper added a comment.
Ping @echristo
https://reviews.llvm.org/D46541
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
craig.topper added a comment.
This looks pretty good to me. @echristo what do you think?
https://reviews.llvm.org/D46541
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
GBuella updated this revision to Diff 145876.
https://reviews.llvm.org/D46541
Files:
lib/CodeGen/CodeGenFunction.cpp
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/CodeGenModule.h
test/CodeGen/target-features-error-2.c
test/CodeGen/target-features-error.c
Index: test/CodeGen/target-features
craig.topper added inline comments.
Comment at: lib/CodeGen/CodeGenFunction.cpp:2342
// Only positive features are "required".
- if (F.getValue())
+ if (F.getValue()) {
+if (std::any_of(ParsedAttr.Features.begin(), ParsedAttr.Features.end(),
-
craig.topper added inline comments.
Comment at: lib/CodeGen/CodeGenFunction.cpp:2346
+ return Feat.substr(1) == F.getKey();
+ }))
+ReqFeatures.insert(ReqFeatures.begin(), F.getKey());
This and the next line a
GBuella updated this revision to Diff 145680.
https://reviews.llvm.org/D46541
Files:
lib/CodeGen/CodeGenFunction.cpp
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/CodeGenModule.h
test/CodeGen/target-features-error-2.c
test/CodeGen/target-features-error.c
Index: test/CodeGen/target-features
craig.topper added inline comments.
Comment at: lib/CodeGen/CodeGenFunction.cpp:2271
+static bool hasRequiredFeature(StringRef Feature,
+ llvm::StringMap& CallerFeatureMap,
+ std::string &Missing) {
'&'
GBuella created this revision.
GBuella added reviewers: craig.topper, echristo, dblaikie.
Herald added a subscriber: cfe-commits.
When requirement imposed by __target__ attributes on functions
are not satisfied, prefer printing those requirements, which
are explicitly mentioned in the attributes.
20 matches
Mail list logo
