See details in comment https://reviews.llvm.org/D135097#3859893 and https://reviews.llvm.org/D135097#3859917
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
@saugustine A, @jrtc27. Thanks for your comments.
Adding '-target x86_64' is restrictive, but not sure what option to use to express that I want target with zero as default address. @aaron.ballman what do you think?
I've added some more codegen reviewers; I think we usually try to avoid CodeGen tests that rely on specific optimization levels.
clang/test/CodeGen/func-attr.c | ||
---|---|---|
1–2 | It isn't really appropriate to add an opt-level to the test, and it doesn't really depend on it. I don't really understand @saugustine 's request in the other thread: MOST of the clang-codegen tests aren't supposed to have opt-levels added to them, and will fail because of it. So I'm unshocked that adding '-O2' to a test would cause it to fail. Clang tests are generally NOT supposed to be run with an opt-setting. |
clang/test/CodeGen/func-attr.c | ||
---|---|---|
1–2 | I did notice that the there is some extra information generated before the attribute at the function define when Ox (x>0) is used. |
clang/test/CodeGen/func-attr.c | ||
---|---|---|
1–2 | I don't believe that is unexpected. The wildcard there is likely fine (though you might want to remove a space either before or after it in case that is not generated), or add --disable-llvm-passes to this if the original patch would only really 'work' at certain opt-levels, but having it run opt is going to cause problems downstream, and makes these tests very sensitive to what the middle/backends do. But running opt on a CodeGen test isn't really appropriate. |
With these changes in place, this looks fine to me.
The underlying problem here is that portable IR generation has gotten too difficult to test textually. FileCheck mostly works fine for testing IR passes, where we rarely worry about target dependencies and carefully control the input, but there are too many variables in IR generation these days that result in too much variation in the output. The result is that our test suite overwhelmingly tests specific targets instead of striving for portability. Ah well.
It isn't really appropriate to add an opt-level to the test, and it doesn't really depend on it. I don't really understand @saugustine 's request in the other thread: MOST of the clang-codegen tests aren't supposed to have opt-levels added to them, and will fail because of it.
So I'm unshocked that adding '-O2' to a test would cause it to fail. Clang tests are generally NOT supposed to be run with an opt-setting.