Use builtin __wasm_exception_handling__ macro. NFC

[sbc100]

No description provided.

[sbc100]

Is there some reason why this one is only comparing with WASM_LEGACY and not WASM?

[aheejin]

I think I forgot to update it. But anyway this is not necessary because we define it in the clang preprocessor: https://github.com/llvm/llvm-project/blob/82acc31fd4b26723b61dae76da120c0c649d0b97/clang/lib/Frontend/InitPreprocessor.cpp#L1034-L1035
So if we are to change the name we have to update this too.

[aheejin]

Why do you want to change it? To be consistent with __wasm__?
I don't mind changing it, but this is upstreamed, so we have to change it there as well. The instances are

aheejin@aheejin:~/llvm-git/clang$ grep __WASM_EXCEPTIONS__ * -R
lib/Frontend/InitPreprocessor.cpp:    Builder.defineMacro("__WASM_EXCEPTIONS__");
test/CodeGenCXX/wasm-eh.cpp:// PREPROCESSOR: #define __WASM_EXCEPTIONS__ 1

aheejin@aheejin:~/llvm-git/libcxxabi$ grep __WASM_EXCEPTIONS__ * -R
src/cxa_personality.cpp:#if !defined(__USING_SJLJ_EXCEPTIONS__) && !defined(__WASM_EXCEPTIONS__)
src/cxa_personality.cpp:#else  // __USING_SJLJ_EXCEPTIONS__ || __WASM_EXCEPTIONS_
src/cxa_personality.cpp:#endif // __USING_SJLJ_EXCEPTIONS__ || __WASM_EXCEPTIONS_
src/cxa_personality.cpp:#if defined(__USING_SJLJ_EXCEPTIONS__) || defined(__WASM_EXCEPTIONS__)
src/cxa_personality.cpp:#if defined(__USING_SJLJ_EXCEPTIONS__) || defined(__WASM_EXCEPTIONS__)
src/cxa_personality.cpp:#else  // !__USING_SJLJ_EXCEPTIONS__ && !__WASM_EXCEPTIONS__
src/cxa_personality.cpp:#endif // !__USING_SJLJ_EXCEPTIONS__ && !__WASM_EXCEPTIONS__
src/cxa_personality.cpp:#if defined(__USING_SJLJ_EXCEPTIONS__) || defined(__WASM_EXCEPTIONS__)
src/cxa_personality.cpp:#if !defined(__USING_SJLJ_EXCEPTIONS__) && !defined(__WASM_EXCEPTIONS__)
src/cxa_personality.cpp:#else  // __USING_SJLJ_EXCEPTIONS__ || __WASM_EXCEPTIONS_
src/cxa_personality.cpp:#endif // __USING_SJLJ_EXCEPTIONS__ || __WASM_EXCEPTIONS_
src/cxa_personality.cpp:#if !defined(__USING_SJLJ_EXCEPTIONS__) && !defined(__WASM_EXCEPTIONS__)
src/cxa_personality.cpp:#else  // __USING_SJLJ_EXCEPTIONS__ || __WASM_EXCEPTIONS_
src/cxa_personality.cpp:#endif // __USING_SJLJ_EXCEPTIONS__ || __WASM_EXCEPTIONS_
src/cxa_personality.cpp:#if !defined(__USING_SJLJ_EXCEPTIONS__) && !defined(__WASM_EXCEPTIONS__)
src/cxa_personality.cpp:#endif // !__USING_SJLJ_EXCEPTIONS__ && !__WASM_EXCEPTIONS__
src/cxa_personality.cpp:#ifdef __WASM_EXCEPTIONS__
src/cxa_personality.cpp:#ifdef __WASM_EXCEPTIONS__

aheejin@aheejin:~/llvm-git/libunwind$ grep __WASM_EXCEPTIONS__ * -R
src/Unwind-wasm.c:#ifdef __WASM_EXCEPTIONS__
src/Unwind-wasm.c:#endif // defined(__WASM_EXCEPTIONS__)

Landing this before changing https://github.com/llvm/llvm-project/blob/82acc31fd4b26723b61dae76da120c0c649d0b97/clang/lib/Frontend/InitPreprocessor.cpp#L1034-L1035 will break the waterfall.

[aheejin]

I think I forgot to update it. But anyway this is not necessary because we define it in the clang preprocessor: https://github.com/llvm/llvm-project/blob/82acc31fd4b26723b61dae76da120c0c649d0b97/clang/lib/Frontend/InitPreprocessor.cpp#L1034-L1035
So if we are to change the name we have to update this too.

[sbc100]

Oh, if __WASM_EXCEPTIONS__ is defined by the pre-processor then maybe I'll instead just remove the setting of it here

[aheejin]

Oh, if __WASM_EXCEPTIONS__ is defined by the pre-processor then maybe I'll instead just remove the setting of it here

But don't you need the library part still?

[sbc100]

Oh, if __WASM_EXCEPTIONS__ is defined by the pre-processor then maybe I'll instead just remove the setting of it here

But don't you need the library part still?

Which part are you referring to?

[aheejin]

That you changed #ifdef __WASM_EXCEPTIONS__ to #ifdef __wasm_exception_handling__ in libraries.

[sbc100]

That you changed #ifdef __WASM_EXCEPTIONS__ to #ifdef __wasm_exception_handling__ in libraries.

On further consideration (and on finding out that __WASM_EXCEPTIONS__ is builtin) I don't think we should do that.

__wasm_exception_handling__ is the feature flag which I think can be enabled even when wasm C++ exception handling is not enabled, eight?

[aheejin]

Not sure if I understand. You mean we should keep __WASM_EXCEPTIONS__ in libraries? That I don't particularly mind, but what do you mean that it is a builtin? It is a preprocessor defined in clang when -fwasm-exceptions is used. Also not sure what you mean by __wasm_exception_handling__ is a feature flag that can be enabled when C++ EH is not used.

(Also llvm/llvm-project#159143 currently uses __wasm_exception_handling__)

[sbc100]

Not sure if I understand. You mean we should keep __WASM_EXCEPTIONS__ in libraries? That I don't particularly mind, but what do you mean that it is a builtin? It is a preprocessor defined in clang when -fwasm-exceptions is used.

By "builtin" I mean "defined by the compiler", yes.

Also not sure what you mean by __wasm_exception_handling__ is a feature flag that can be enabled when C++ EH is not used.

I mean could enable the wasm exception handling feature while not enabled wasm C++ EH, couldn't you? At least there seem to be two difference compiler-defined macros at play here.

(Also llvm/llvm-project#159143 currently uses __wasm_exception_handling__)

Yes, because I want to define those tags whenever wasm exceptions handling is enabled, regardless of -fwasm-exceptions which, for example, might not make sense when compiling a .S file.

[aheejin]

Do you mean we should keep two different preprocessors, __was_exception_handling__ for Wasm EH feature and __WASM_EXCEPTIONS for -fwasm-exceptions? That seems confusing... Can't we use a single one for that? Why would we need a preprocessor for Wasm EH feature? If you'd like to have a preprocessor that can also work for SjLj, we have __WASM_SJLJ__ for it. If we add one more preprocessor, we have three prepreocessors for Wasm EH/SjLj.

We talked about merging __WASM_EXCEPTION__ and __WASM_SJLJ__ but decided not to do it in #21971.

[sbc100]

Do you mean we should keep two different preprocessors, __was_exception_handling__ for Wasm EH feature and __WASM_EXCEPTIONS for -fwasm-exceptions? That seems confusing... Can't we use a single one for that? Why would we need a preprocessor for Wasm EH feature? If you'd like to have a preprocessor that can also work for SjLj, we have __WASM_SJLJ__ for it. If we add one more preprocessor, we have three prepreocessors for Wasm EH/SjLj.

I believe each wasm feature gets its own preprocess definition. This means that __wasm_exception_handling__ is enabled automatically whenever the wasm EH is available. For example, this can be defined in C files (not just C++ files) or assembly files or even in C++ files with with -fno-exceptions. I think this makes sense because in theory wasm EH could be used for things other than C++ EH.

__WASM_EXCEPTION__ on the other hand seems to be specifically related to the C++ EH model and can be controlled via different, flags right?

If you think this is not needed then I suggest you propose removing one of them. Removing __wasm_exception_handling__ seems hard since that is automatic, based on the wasm feature set that is enabled.

We talked about merging __WASM_EXCEPTION__ and __WASM_SJLJ__ but decided not to do it in #21971.