Skip to content

[WebAssembly][GlobalISel] Fix legalizeCustom return value for Helper.lower()#191345

Merged
tclin914 merged 2 commits intollvm:mainfrom
tclin914:fix-wasm-legalizecustom-return
Apr 10, 2026
Merged

[WebAssembly][GlobalISel] Fix legalizeCustom return value for Helper.lower()#191345
tclin914 merged 2 commits intollvm:mainfrom
tclin914:fix-wasm-legalizecustom-return

Conversation

@tclin914
Copy link
Copy Markdown
Contributor

Helper.lower() returns a LegalizerHelper::LegalizeResult enum where UnableToLegalize=2, which implicitly converts to true (success). Compare against LegalizerHelper::Legalized instead so that legalization failures are correctly reported.

…lower()

Helper.lower() returns a LegalizerHelper::LegalizeResult enum where
UnableToLegalize=2, which implicitly converts to true (success). Compare
against LegalizerHelper::Legalized instead so that legalization failures
are correctly reported.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@llvmbot
Copy link
Copy Markdown
Member

llvmbot commented Apr 10, 2026

@llvm/pr-subscribers-backend-webassembly

Author: Jim Lin (tclin914)

Changes

Helper.lower() returns a LegalizerHelper::LegalizeResult enum where UnableToLegalize=2, which implicitly converts to true (success). Compare against LegalizerHelper::Legalized instead so that legalization failures are correctly reported.


Full diff: https://github.com/llvm/llvm-project/pull/191345.diff

1 Files Affected:

  • (modified) llvm/lib/Target/WebAssembly/GISel/WebAssemblyLegalizerInfo.cpp (+1-1)
diff --git a/llvm/lib/Target/WebAssembly/GISel/WebAssemblyLegalizerInfo.cpp b/llvm/lib/Target/WebAssembly/GISel/WebAssemblyLegalizerInfo.cpp
index 293ba88605354..a902fcfea6c38 100644
--- a/llvm/lib/Target/WebAssembly/GISel/WebAssemblyLegalizerInfo.cpp
+++ b/llvm/lib/Target/WebAssembly/GISel/WebAssemblyLegalizerInfo.cpp
@@ -90,7 +90,7 @@ bool WebAssemblyLegalizerInfo::legalizeCustom(
       return true;
     }
 
-    return Helper.lower(MI, 0, DstType);
+    return Helper.lower(MI, 0, DstType) == LegalizerHelper::Legalized;
   }
   default:
     break;

@QuantumSegfault
Copy link
Copy Markdown
Contributor

Wow! Thanks. How'd you notice?

Also, shouldn't this be != LegalizerHelper::UnableToLegalize. That way we don't return false for AlreadyLegal?

…ized

Use != UnableToLegalize so that AlreadyLegal is also treated as success,
not just Legalized.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@tclin914
Copy link
Copy Markdown
Contributor Author

Wow! Thanks. How'd you notice?

Also, shouldn't this be != LegalizerHelper::UnableToLegalize. That way we don't return false for AlreadyLegal?

Updated to != UnableToLegalize. I've been experimenting with AI-assisted code review on recent commits — it flagged the implicit enum-to-bool conversion here where UnableToLegalize=2 converts to true.

@tclin914 tclin914 merged commit f95ed25 into llvm:main Apr 10, 2026
10 checks passed
@tclin914 tclin914 deleted the fix-wasm-legalizecustom-return branch April 10, 2026 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants