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

[SPIR-V] Fix 64-bit integer literal printing #66686

Merged
merged 1 commit into from
Sep 20, 2023

Conversation

sudonatalie
Copy link
Member

Previously, the SPIR-V instruction printer was always printing the first operand of an OpConstant's literal value as one of the fixed operands. This is incorrect for 64-bit values, where the first operand is actually the value's lower-order word and should be combined with the following higher-order word before printing.

This change fixes that issue by waiting to print the last fixed operand of OpConstant instructions until the variadic operands are ready to be printed, then using NumFixedOps - 1 as the starting operand index for the literal value operands.

Depends on D156049

Previously, the SPIR-V instruction printer was always printing the first
operand of an OpConstant's variable value as one of the fixed operands.
This is incorrect for 64-bit values, where the first operand is actually
the value's lower word and should be combined with the following higher
order word before printing.

This change fixes that issue by waiting to print the last fixed operand
of OpConstant instructions until the variadic operands are ready to be
printed, then using NumFixedOps - 1 as the starting operand index for
the variable value operands.

Depends on D156049
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 18, 2023

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

Changes

Previously, the SPIR-V instruction printer was always printing the first operand of an OpConstant's literal value as one of the fixed operands. This is incorrect for 64-bit values, where the first operand is actually the value's lower-order word and should be combined with the following higher-order word before printing.

This change fixes that issue by waiting to print the last fixed operand of OpConstant instructions until the variadic operands are ready to be printed, then using NumFixedOps - 1 as the starting operand index for the literal value operands.

Depends on D156049


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

6 Files Affected:

  • (modified) llvm/lib/Target/SPIRV/MCTargetDesc/SPIRVInstPrinter.cpp (+3-1)
  • (modified) llvm/lib/Target/SPIRV/SPIRVInstrInfo.td (+2-2)
  • (modified) llvm/test/CodeGen/SPIRV/atomicrmw.ll (+2-2)
  • (modified) llvm/test/CodeGen/SPIRV/constant/local-integers-constants.ll (+2-2)
  • (modified) llvm/test/CodeGen/SPIRV/lshr-constexpr.ll (+1-1)
  • (modified) llvm/test/CodeGen/SPIRV/uitofp-with-bool.ll (+2-2)
diff --git a/llvm/lib/Target/SPIRV/MCTargetDesc/SPIRVInstPrinter.cpp b/llvm/lib/Target/SPIRV/MCTargetDesc/SPIRVInstPrinter.cpp
index a1e90cd104a9708..e472f7bcbaa50db 100644
--- a/llvm/lib/Target/SPIRV/MCTargetDesc/SPIRVInstPrinter.cpp
+++ b/llvm/lib/Target/SPIRV/MCTargetDesc/SPIRVInstPrinter.cpp
@@ -169,7 +169,9 @@ void SPIRVInstPrinter::printInst(const MCInst *MI, uint64_t Address,
         }
         case SPIRV::OpConstantI:
         case SPIRV::OpConstantF:
-          printOpConstantVarOps(MI, NumFixedOps, OS);
+          // The last fixed operand along with any variadic operands that follow
+          // are part of the variable value.
+          printOpConstantVarOps(MI, NumFixedOps - 1, OS);
           break;
         default:
           printRemainingVariableOps(MI, NumFixedOps, OS);
diff --git a/llvm/lib/Target/SPIRV/SPIRVInstrInfo.td b/llvm/lib/Target/SPIRV/SPIRVInstrInfo.td
index 44b5536becf7f4b..5f3bc30591fa590 100644
--- a/llvm/lib/Target/SPIRV/SPIRVInstrInfo.td
+++ b/llvm/lib/Target/SPIRV/SPIRVInstrInfo.td
@@ -217,9 +217,9 @@ def ConstPseudoNull: IntImmLeaf<i64, [{ return Imm.isZero(); }]>;
 
 multiclass IntFPImm<bits<16> opCode, string name> {
   def I: Op<opCode, (outs ID:$dst), (ins TYPE:$type, ID:$src, variable_ops),
-                  "$dst = "#name#" $type $src", [(set ID:$dst, (assigntype PseudoConstI:$src, TYPE:$type))]>;
+                  "$dst = "#name#" $type", [(set ID:$dst, (assigntype PseudoConstI:$src, TYPE:$type))]>;
   def F: Op<opCode, (outs ID:$dst), (ins TYPE:$type, fID:$src, variable_ops),
-                  "$dst = "#name#" $type $src", [(set ID:$dst, (assigntype PseudoConstF:$src, TYPE:$type))]>;
+                  "$dst = "#name#" $type", [(set ID:$dst, (assigntype PseudoConstF:$src, TYPE:$type))]>;
 }
 
 def OpConstantTrue: Op<41, (outs ID:$dst), (ins TYPE:$src_ty), "$dst = OpConstantTrue $src_ty",
diff --git a/llvm/test/CodeGen/SPIRV/atomicrmw.ll b/llvm/test/CodeGen/SPIRV/atomicrmw.ll
index de192d5bd422d85..dc964e749844560 100644
--- a/llvm/test/CodeGen/SPIRV/atomicrmw.ll
+++ b/llvm/test/CodeGen/SPIRV/atomicrmw.ll
@@ -1,10 +1,10 @@
 ; RUN: llc -O0 -opaque-pointers=0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s
 
 ; CHECK:     %[[#Int:]] = OpTypeInt 32 0
-; CHECK-DAG: %[[#Scope_Device:]] = OpConstant %[[#Int]] 1 {{$}}
+; CHECK-DAG: %[[#Scope_Device:]] = OpConstant %[[#Int]] 1{{$}}
 ; CHECK-DAG: %[[#MemSem_Relaxed:]] = OpConstant %[[#Int]] 0
 ; CHECK-DAG: %[[#MemSem_Acquire:]] = OpConstant %[[#Int]] 2
-; CHECK-DAG: %[[#MemSem_Release:]] = OpConstant %[[#Int]] 4 {{$}}
+; CHECK-DAG: %[[#MemSem_Release:]] = OpConstant %[[#Int]] 4{{$}}
 ; CHECK-DAG: %[[#MemSem_AcquireRelease:]] = OpConstant %[[#Int]] 8
 ; CHECK-DAG: %[[#MemSem_SequentiallyConsistent:]] = OpConstant %[[#Int]] 16
 ; CHECK-DAG: %[[#Value:]] = OpConstant %[[#Int]] 42
diff --git a/llvm/test/CodeGen/SPIRV/constant/local-integers-constants.ll b/llvm/test/CodeGen/SPIRV/constant/local-integers-constants.ll
index 9660ba446cfca43..5cfc0d2e9dc8f49 100644
--- a/llvm/test/CodeGen/SPIRV/constant/local-integers-constants.ll
+++ b/llvm/test/CodeGen/SPIRV/constant/local-integers-constants.ll
@@ -44,8 +44,8 @@ define i64 @getLargeConstantI64() {
 ; CHECK-DAG: %[[#CST_I8:]] = OpConstant %[[#I8]] 2
 ; CHECK-DAG: %[[#CST_I16:]] = OpConstant %[[#I16]] 65478
 ; CHECK-DAG: %[[#CST_I32:]] = OpConstant %[[#I32]] 42
-; CHECK-DAG: %[[#CST_I64:]] = OpConstant %[[#I64]] 123456789 0
-; CHECK-DAG: %[[#CST_LARGE_I64:]] = OpConstant %[[#I64]] 0 8
+; CHECK-DAG: %[[#CST_I64:]] = OpConstant %[[#I64]] 123456789
+; CHECK-DAG: %[[#CST_LARGE_I64:]] = OpConstant %[[#I64]] 34359738368
 
 ; CHECK: %[[#GET_I8]] = OpFunction %[[#I8]]
 ; CHECK: OpReturnValue %[[#CST_I8]]
diff --git a/llvm/test/CodeGen/SPIRV/lshr-constexpr.ll b/llvm/test/CodeGen/SPIRV/lshr-constexpr.ll
index c435cb25ef5c2fc..44b11f3ddb27314 100644
--- a/llvm/test/CodeGen/SPIRV/lshr-constexpr.ll
+++ b/llvm/test/CodeGen/SPIRV/lshr-constexpr.ll
@@ -5,7 +5,7 @@
 ; CHECK-SPIRV:     %[[#type_vec:]] = OpTypeVector %[[#type_int32]] 2
 ; CHECK-SPIRV:     %[[#const1:]] = OpConstant %[[#type_int32]] 1
 ; CHECK-SPIRV:     %[[#vec_const:]] = OpConstantComposite %[[#type_vec]] %[[#const1]] %[[#const1]]
-; CHECK-SPIRV:     %[[#const32:]] = OpConstant %[[#type_int64]] 32 0
+; CHECK-SPIRV:     %[[#const32:]] = OpConstant %[[#type_int64]] 32
 
 ; CHECK-SPIRV:     %[[#bitcast_res:]] = OpBitcast %[[#type_int64]] %[[#vec_const]]
 ; CHECK-SPIRV:     %[[#shift_res:]] = OpShiftRightLogical %[[#type_int64]] %[[#bitcast_res]] %[[#const32]]
diff --git a/llvm/test/CodeGen/SPIRV/uitofp-with-bool.ll b/llvm/test/CodeGen/SPIRV/uitofp-with-bool.ll
index 75997a37bafffb0..2fe0dc91e7b4ca2 100644
--- a/llvm/test/CodeGen/SPIRV/uitofp-with-bool.ll
+++ b/llvm/test/CodeGen/SPIRV/uitofp-with-bool.ll
@@ -41,10 +41,10 @@
 ; SPV-DAG: %[[#mone_16:]] = OpConstant %[[#int_16]] 65535
 ; SPV-DAG: %[[#mone_32:]] = OpConstant %[[#int_32]] 4294967295
 ; SPV-DAG: %[[#zero_64:]] = OpConstantNull %[[#int_64]]
-; SPV-DAG: %[[#mone_64:]] = OpConstant %[[#int_64]] 4294967295 4294967295
+; SPV-DAG: %[[#mone_64:]] = OpConstant %[[#int_64]] 18446744073709551615
 ; SPV-DAG: %[[#one_8:]] = OpConstant %[[#int_8]] 1
 ; SPV-DAG: %[[#one_16:]] = OpConstant %[[#int_16]] 1
-; SPV-DAG: %[[#one_64:]] = OpConstant %[[#int_64]] 1 0
+; SPV-DAG: %[[#one_64:]] = OpConstant %[[#int_64]] 1
 ; SPV-DAG: %[[#void:]] = OpTypeVoid
 ; SPV-DAG: %[[#float:]] = OpTypeFloat 32
 ; SPV-DAG: %[[#bool:]] = OpTypeBool

@sudonatalie sudonatalie merged commit f7bfa58 into llvm:main Sep 20, 2023
3 checks passed
@sudonatalie sudonatalie deleted the literal-ints branch September 20, 2023 13:31
sudonatalie added a commit that referenced this pull request Sep 20, 2023
Print OpConstant floats as formatted decimal floating points, with
special case exceptions to print infinity and NaN as hexfloats.

This change follows from the fixes in
#66686 to correct how
constant values are printed generally.

Differential Revision: https://reviews.llvm.org/D159376
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.

None yet

3 participants