Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPIRV] Fix return type mismatch for createSPIRVEmitNonSemanticDIPass #105889

Merged
merged 2 commits into from
Aug 23, 2024

Conversation

bogner
Copy link
Contributor

@bogner bogner commented Aug 23, 2024

The declaration in SPIRV.h had this returning a MachineFunctionPass *, but the implementation returned a FunctionPass *. This showed up as a build error on windows, but it was clearly a mistake regardless.

I also updated the pass to include SPIRV.h rather than using its own declarations for pass initialization, as this results in better errors for this kind of typo.

Fixes a build break after #97558

The declaration in SPIRV.h had this returning a `MachineFunctionPass
*`, but the implementation returned a `FunctionPass *`. This showed up
as a build error on windows, but it was clearly a mistake regardless.

I also updated the pass to include SPIRV.h rather than using its own
declarations for pass initialization, as this results in better errors
for this kind of typo.
@llvmbot
Copy link
Member

llvmbot commented Aug 23, 2024

@llvm/pr-subscribers-backend-spir-v

Author: Justin Bogner (bogner)

Changes

The declaration in SPIRV.h had this returning a MachineFunctionPass *, but the implementation returned a FunctionPass *. This showed up as a build error on windows, but it was clearly a mistake regardless.

I also updated the pass to include SPIRV.h rather than using its own declarations for pass initialization, as this results in better errors for this kind of typo.


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

1 Files Affected:

  • (modified) llvm/lib/Target/SPIRV/SPIRVEmitNonSemanticDI.cpp (+6-6)
diff --git a/llvm/lib/Target/SPIRV/SPIRVEmitNonSemanticDI.cpp b/llvm/lib/Target/SPIRV/SPIRVEmitNonSemanticDI.cpp
index cc506356e39043..01c03215fb9e75 100644
--- a/llvm/lib/Target/SPIRV/SPIRVEmitNonSemanticDI.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVEmitNonSemanticDI.cpp
@@ -1,3 +1,4 @@
+#include "SPIRV.h"
 #include "MCTargetDesc/SPIRVBaseInfo.h"
 #include "MCTargetDesc/SPIRVMCTargetDesc.h"
 #include "SPIRVGlobalRegistry.h"
@@ -33,12 +34,6 @@ struct SPIRVEmitNonSemanticDI : public MachineFunctionPass {
   bool IsGlobalDIEmitted = false;
   bool emitGlobalDI(MachineFunction &MF);
 };
-
-void initializeSPIRVEmitNonSemanticDIPass(PassRegistry &);
-
-FunctionPass *createSPIRVEmitNonSemanticDIPass(SPIRVTargetMachine *TM) {
-  return new SPIRVEmitNonSemanticDI(TM);
-}
 } // namespace llvm
 
 using namespace llvm;
@@ -48,6 +43,11 @@ INITIALIZE_PASS(SPIRVEmitNonSemanticDI, DEBUG_TYPE,
 
 char SPIRVEmitNonSemanticDI::ID = 0;
 
+MachineFunctionPass *
+llvm::createSPIRVEmitNonSemanticDIPass(SPIRVTargetMachine *TM) {
+  return new SPIRVEmitNonSemanticDI(TM);
+}
+
 SPIRVEmitNonSemanticDI::SPIRVEmitNonSemanticDI(SPIRVTargetMachine *TM)
     : MachineFunctionPass(ID), TM(TM) {
   initializeSPIRVEmitNonSemanticDIPass(*PassRegistry::getPassRegistry());

Copy link

github-actions bot commented Aug 23, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Member

@michalpaszkowski michalpaszkowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bogner Thanks for catching this and the fix!

@bogner bogner merged commit 3e763db into llvm:main Aug 23, 2024
6 of 9 checks passed
5chmidti pushed a commit that referenced this pull request Aug 24, 2024
…#105889)

The declaration in SPIRV.h had this returning a `MachineFunctionPass *`,
but the implementation returned a `FunctionPass *`. This showed up as a
build error on windows, but it was clearly a mistake regardless.

I also updated the pass to include SPIRV.h rather than using its own
declarations for pass initialization, as this results in better errors
for this kind of typo.

Fixes a build break after #97558
bogner added a commit that referenced this pull request Aug 26, 2024
…#105965)

I don't know if this is the right thing to do or if the emitter should
be normalizing path separators, but this gets this test to pass on
windows where `sys::path::append` will use "\" instead of "/".

This is another follow up to #97558, as #105889 managed to fix the
build, but not all of the tests.
5c4lar pushed a commit to 5c4lar/llvm-project that referenced this pull request Aug 29, 2024
…llvm#105965)

I don't know if this is the right thing to do or if the emitter should
be normalizing path separators, but this gets this test to pass on
windows where `sys::path::append` will use "\" instead of "/".

This is another follow up to llvm#97558, as llvm#105889 managed to fix the
build, but not all of the tests.
dmpolukhin pushed a commit to dmpolukhin/llvm-project that referenced this pull request Sep 2, 2024
…llvm#105889)

The declaration in SPIRV.h had this returning a `MachineFunctionPass *`,
but the implementation returned a `FunctionPass *`. This showed up as a
build error on windows, but it was clearly a mistake regardless.

I also updated the pass to include SPIRV.h rather than using its own
declarations for pass initialization, as this results in better errors
for this kind of typo.

Fixes a build break after llvm#97558
dmpolukhin pushed a commit to dmpolukhin/llvm-project that referenced this pull request Sep 2, 2024
…llvm#105965)

I don't know if this is the right thing to do or if the emitter should
be normalizing path separators, but this gets this test to pass on
windows where `sys::path::append` will use "\" instead of "/".

This is another follow up to llvm#97558, as llvm#105889 managed to fix the
build, but not all of the tests.
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