Skip to content

Conversation

@jsji
Copy link
Member

@jsji jsji commented Oct 18, 2024

This is one of the many PRs to fix errors with LLVM_ENABLE_WERROR=on. Built by GCC 11.

Fix warnings:
llvm-project/llvm/lib/Target/X86/X86FastISel.cpp: In member function ‘virtual bool {anonymous}::X86FastISel::fastLowerCall(llvm::FastISel::CallLoweringInfo&)’:
llvm-project/llvm/lib/Target/X86/X86FastISel.cpp:3547: error: enumerated and non-enumerated type in conditional expression [-Werror=extra]
3547 | MIB.addReg(Is64Bit ? X86::RIP : 0).addImm(1).addReg(0);

This is one of the many PRs to fix errors with LLVM_ENABLE_WERROR=on. Built by GCC 11.

Fix warnings:
llvm-project/llvm/lib/Target/X86/X86FastISel.cpp: In member function ‘virtual bool {anonymous}::X86FastISel::fastLowerCall(llvm::FastISel::CallLoweringInfo&)’:
llvm-project/llvm/lib/Target/X86/X86FastISel.cpp:3547: error: enumerated and non-enumerated type in conditional expression [-Werror=extra]
 3547 |       MIB.addReg(Is64Bit ? X86::RIP : 0).addImm(1).addReg(0);
@jsji jsji self-assigned this Oct 18, 2024
@jsji jsji requested a review from phoebewang October 18, 2024 02:53
@llvmbot
Copy link
Member

llvmbot commented Oct 18, 2024

@llvm/pr-subscribers-backend-x86

Author: Jinsong Ji (jsji)

Changes

This is one of the many PRs to fix errors with LLVM_ENABLE_WERROR=on. Built by GCC 11.

Fix warnings:
llvm-project/llvm/lib/Target/X86/X86FastISel.cpp: In member function ‘virtual bool {anonymous}::X86FastISel::fastLowerCall(llvm::FastISel::CallLoweringInfo&)’:
llvm-project/llvm/lib/Target/X86/X86FastISel.cpp:3547: error: enumerated and non-enumerated type in conditional expression [-Werror=extra]
3547 | MIB.addReg(Is64Bit ? X86::RIP : 0).addImm(1).addReg(0);


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

2 Files Affected:

  • (modified) llvm/lib/Target/X86/X86FastISel.cpp (+1-1)
  • (modified) llvm/lib/Target/X86/X86ISelLowering.cpp (+1-1)
diff --git a/llvm/lib/Target/X86/X86FastISel.cpp b/llvm/lib/Target/X86/X86FastISel.cpp
index 4bf660b5e234ae..9e4e5547c642cc 100644
--- a/llvm/lib/Target/X86/X86FastISel.cpp
+++ b/llvm/lib/Target/X86/X86FastISel.cpp
@@ -3544,7 +3544,7 @@ bool X86FastISel::fastLowerCall(CallLoweringInfo &CLI) {
 
     MIB = BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, TII.get(CallOpc));
     if (NeedLoad)
-      MIB.addReg(Is64Bit ? X86::RIP : 0).addImm(1).addReg(0);
+      MIB.addReg(Is64Bit ? X86::RIP : X86::NoRegister).addImm(1).addReg(0);
     if (Symbol)
       MIB.addSym(Symbol, OpFlags);
     else
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index db633d10edc49a..a469a5a554354f 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -29986,7 +29986,7 @@ static SDValue LowerShift(SDValue Op, const X86Subtarget &Subtarget,
           MVT::getVectorVT(NarrowScalarVT, WideNumElts), dl, AmtWideElts);
       AmtWide = DAG.getZExtOrTrunc(AmtWide, dl, WideVT);
       // Perform the actual shift.
-      unsigned LogicalOpc = Opc == ISD::SRA ? ISD::SRL : Opc;
+      unsigned LogicalOpc = Opc == ISD::SRA ? (unsigned)ISD::SRL : Opc;
       SDValue ShiftedR = DAG.getNode(LogicalOpc, dl, WideVT, RWide, AmtWide);
       // Now we need to construct a mask which will "drop" bits that get
       // shifted past the LSB/MSB. For a logical shift left, it will look

AmtWide = DAG.getZExtOrTrunc(AmtWide, dl, WideVT);
// Perform the actual shift.
unsigned LogicalOpc = Opc == ISD::SRA ? ISD::SRL : Opc;
unsigned LogicalOpc = Opc == ISD::SRA ? (unsigned)ISD::SRL : Opc;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why need (unsigned) here? If the ISD::XXX is not a unsigned value, I think we would have more warn/error when comparing with Opc.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, maybe because we just use == and != here.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is about Opc is unsigned and ISD::SRL is enum, and they are used in the ?: operator, so type mismatch.

Copy link
Contributor

@phoebewang phoebewang left a comment

Choose a reason for hiding this comment

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

LGTM.

@jsji jsji merged commit d7ae43e into llvm:main Oct 18, 2024
10 checks passed
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