misattributed warning line when forgetting to use `-fopencilk`

[wheatman]

The following code

#include <cilk/cilk.h>

int main() {

  cilk_for(int i = 0; i < 10; i++) {}

  return 0;
}

gives the warning of

del.cpp:3:5: warning: CodeGen found Tapir instructions to serialize.  Specify a Tapir back-end to lower Tapir instructions to a parallel runtime. [-Wpass-failed=tapircleanup]
int main() {
    ^
1 warning generated.

I would expect it to point at the line with the cilk_for, not the start of the function.

The same behavior also happens when using cilk_spawn as in

#include <cilk/cilk.h>

void foo() {}

int main() {

  cilk_spawn foo();

  return 0;
}

giving a warning of

del.cpp:5:5: warning: CodeGen found Tapir instructions to serialize.  Specify a Tapir back-end to lower Tapir instructions to a parallel runtime. [-Wpass-failed=tapircleanup]
int main() {
    ^
1 warning generated.

[neboat]

There's no good way to fix this issue. The misattribution here is happening when the code is compiled with no debug symbols. Compiling with -g resolves this issue, but without any debug symbols, there isn't a way to attribute the error to something more granular than the function itself.

[wheatman]

Two thoughts, but I understand that this is probably not a very important issue. Could you use similar mechanism like that in -Wc++20-compat which is able to diagnose just the use of a keyword https://godbolt.org/z/9GMz4chr3 ?

Could the warning text get a note that it is probably the use of a cilk_for or cilk_spwan command? People may use cilk without knowing what Tapir is

[VoxSciurorum]

We offer two different headers, <cilk/cilk.h> to enable keywords and <cilk/cilk_stub.h> to disable keywords. Would it make more sense to have one header with behavior controled by a macro like __cilk?

[neboat]

I don't see how modifying the headers or macro definitions affects this specific warning, which is coming from the back-end of the compiler, not the clang front-end.

We might be able to introduce a different warning that clang itself would report. IIRC, we already use something like a __cilk macro to interact properly with Cilk Plus header files, so repurposing that macro or introducing a new one should be feasible. But I'm concerned with what impact such new checks and warning might have on using OpenCilk, especially with different edge cases of compiling and linking OpenCilk code. I don't think it's worth delaying the next OpenCilk release to work through all of those edge cases.

[wheatman]

I would agree that this is not an urgent issue and would rather get the new version without this fix then wait for it.

I guess trying to clarify what I think a a good fix in the long term would be, though a relatively low importance .

If a user specifies a cilk keyword and does not compile with -fopencilk they should get an error that points them at the use and tells them to add the flag. Also, the error should include the keyword they used, and not tapir or things with random extra underscores

I wonder if this could actually go a step further and be a warning instead of an error, and the serial semantics would automatically just work.

[VoxSciurorum]

If the cilk_for keyword turns to for when -fopencilk is not used there will be nothing for the backend to complain about.

[neboat]

That behavior sounds like it may cause problems for our infrastructure for regression-testing Clang, as well as for build systems that try to incorporate OpenCilk. We would need to chase down those issues to verify that the change doesn't cause other problems.

To a lesser extent, I'm also concerned with the impact that change would have on people developing alternative front-ends to the compiler. If the alternative front-ends are sidestepping Clang altogether, then this change wouldn't matter, but we might have problems with new front-ends that build on top of Clang.