[PATCH] D45559: [WebAssembly] Add Wasm personality and usesWindowsEHInstructions()

[Derek Schuff via Phabricator via llvm-commits]

dschuff added inline comments.


================
Comment at: include/llvm/Analysis/EHPersonalities.h:30
   GNU_CXX_SjLj,
+  GNU_CXX_Wasm,
   GNU_ObjC,
----------------
We might just want to call this 'Wasm',  since it will be sort of a hybrid between MSVC (IR) and GNU (library API). Does the personality enum refer to just the personality function itself, or does it describe the scheme more generally?


================
Comment at: include/llvm/Analysis/EHPersonalities.h:80
+/// instructions: catchswitch, catchpad/ret, and cleanuppad/ret.
+inline bool usesWindowsEHInstructions(EHPersonality Pers) {
+  switch (Pers) {
----------------
Generally in this change you've gone from a fairly generic "funclet-based" to a more specific "windows"-related terminology; but of course you aren't doing this for windows; maybe this could be `usesCatchpadInstructions` or `usesCatchpadEH` which would be shorter and still generic.


================
Comment at: lib/Analysis/MustExecute.cpp:55
     if (Constant *PersonalityFn = Fn->getPersonalityFn())
-      if (isFuncletEHPersonality(classifyEHPersonality(PersonalityFn)))
+      if (usesWindowsEHInstructions(classifyEHPersonality(PersonalityFn)))
         SafetyInfo->BlockColors = colorEHFunclets(*Fn);
----------------
do we actually need to run `colorEHFunclets` for wasm?


================
Comment at: lib/CodeGen/DwarfEHPrepare.cpp:198
 
   // Check the personality, don't do anything if it's funclet-based.
   EHPersonality Pers = classifyEHPersonality(Fn.getPersonalityFn());
----------------
Update the comment too (wasm isn't funclet-based)


================
Comment at: lib/CodeGen/WinEHPrepare.cpp:109
 
   // Do nothing if this is not a funclet-based personality.
+  if (!usesWindowsEHInstructions(Personality))
----------------
update comment.


================
Comment at: lib/Transforms/Instrumentation/GCOVProfiling.cpp:553
       if (!functionHasLines(F)) continue;
       // TODO: Functions using funclet-based EH are currently not supported.
+      if (isUsingWindowsEHInstructions(F)) continue;
----------------
update comment


================
Comment at: lib/Transforms/Instrumentation/GCOVProfiling.cpp:632
       if (!functionHasLines(F)) continue;
       // TODO: Functions using funclet-based EH are currently not supported.
+      if (isUsingWindowsEHInstructions(F)) continue;
----------------
comment


================
Comment at: lib/Transforms/Utils/EscapeEnumerator.cpp:77
+  if (usesWindowsEHInstructions(classifyEHPersonality(F.getPersonalityFn()))) {
     report_fatal_error("Funclet EH not supported");
   }
----------------
error text


================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:1572
     EHPersonality Personality = classifyEHPersonality(CallerPersonality);
-    if (isFuncletEHPersonality(Personality)) {
+    if (usesWindowsEHInstructions(Personality)) {
       Optional<OperandBundleUse> ParentFunclet =
----------------
this code is also funclet-related. have you checked that it does what we want?


Repository:
  rL LLVM

https://reviews.llvm.org/D45559