add test for inlinealways function attribute#426
Conversation
| // private function to make code elsewhere easier | ||
| #[llvm_versions(4.0..12.0)] | ||
| fn is_type(self) -> bool { | ||
| false | ||
| } |
There was a problem hiding this comment.
this feels a bit cheaty, but it's the simplest way I saw to keep code duplication to a minimum. Suggestions are welcome.
| let alignstack_attribute = Attribute::get_named_enum_kind_id("alignstack"); | ||
| let enum_attribute = context.create_enum_attribute(alignstack_attribute, 1); |
There was a problem hiding this comment.
alignstack is not a valid attibute on a function return type. At least for llvm 15, any value beside 0 (1 on line 88 here) leads to issues. The attribute is still applied when that value is 0. I think the value is only useful for some attributes (e.g. dereferencable(n), where n is the enum value).
to apply the noalias attribute instead, the function return type must be a pointer type.
| pub(crate) attribute: LLVMAttributeRef, | ||
| } | ||
|
|
||
| impl std::fmt::Debug for Attribute { |
There was a problem hiding this comment.
I used the Debug and PartialEq instance during debugging. I can remove them again, but then when something fails you only see an address, which is not helpful at all.
|
is CI supposed to work for LLVM 16? it seems to fail before it even tries to build/run tests, like LLVM 16 is just not installed on the CI machine. |
|
Yes, 16 was working before. Not sure what changed |
|
well I'm not sure what changed then. The branch is rebased on current |
|
@FawazTirmizi do you think you'd be able to look into LLVM16 suddenly failing in CI? |
|
gently bumping this |
|
Hi, sorry for the delay. The llvm16 build will have to get fixed before this can be merged |
|
yes. Anything I can help with for that? the failure seems kind of random. Maybe we should just retry it? |
|
Yeah, I'll rerun it but I'm not confident it'll fix it |
|
Been looking into this but it's not really clear why it fails for the latest version but not the others.. |
|
Been wondering if llvm-sys changed something in 160 |
|
I added a call to that makes sense actually, the LLVM install action installs |
|
and it just looks like for llvm I'm not sure what the best approach is here. I suspect we can configure this in github CI using something like this maybe + # only use ubuntu-22.04 for llvm 16 and higher
+ include:
+ - os: ubuntu-22.04
+ llvm-version: ["16.0", "16-0"]
+ exclude:
+ - os: ubuntu-20.04
+ llvm-version: ["16.0", "16-0"]not pretty, but might do the trick. I have no idea if this is just a fluke or if llvm (in a point release ?!) decided do bump its minimum supported ubuntu version? an alternative is to force an earlier version of llvm. |
cc00832 to
73d19bc
Compare
|
ugh, that's really annoying if they bumped the ubuntu version in a point release of all times |
|
for point releases, the builds they provide are really spotty. The later point releases have no binaries for ubuntu at all... so what is the best option here
|
|
If all the older versions work on u22 then we should just update to that. I'm assuming that isn't the case, so we probably want to make newer versions start using u22 but keep the older ones on u20 |
73d19bc to
74cbef2
Compare
74cbef2 to
9e13d16
Compare
|
allright, got it to work. Turns out there is a way to do this sort of thing compactly |
|
Oh wow, thank you for looking into it! |
Description
Improve test for attributes on functions. Attributes on the
Functionlocation were not tested at all. Thealignstackattribute was applied to the return type, but this attribute is not actually valid in that position. Averify()on the module called this issue out.Checklist