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

[Backport to 16] [OpaquePointers] Adjust builtin variable tracking to support i8 geps. (#1892) #2060

Merged

Conversation

mateuszchudyk
Copy link
Contributor

The existing logic for the replacement of builtin variables with calls to
functions relies on relatively brittle tracking that is broken when opaque
pointers is turned on, and will be even more thoroughly broken if/when typed
geps are replaced with i8 geps or ptradd. This patch replaces that logic with a
less brittle variant that is able to handle any sequence of bitcast, gep, or
addrspacecast instructions between the global variable and the ultimate load
instruction.

It still will error out if the variable is used in too insane of a fashion (say,
trying to load an i32 out of the i64, or a misaligned vector type).

… support i8 geps. (KhronosGroup#1892)

The existing logic for the replacement of builtin variables with calls to
functions relies on relatively brittle tracking that is broken when opaque
pointers is turned on, and will be even more thoroughly broken if/when typed
geps are replaced with i8 geps or ptradd. This patch replaces that logic with a
less brittle variant that is able to handle any sequence of bitcast, gep, or
addrspacecast instructions between the global variable and the ultimate load
instruction.

It still will error out if the variable is used in too insane of a fashion (say,
trying to load an i32 out of the i64, or a misaligned vector type).
@mateuszchudyk mateuszchudyk changed the base branch from main to llvm_release_160 June 22, 2023 08:01
@karolherbst
Copy link
Contributor

Just wanted to point out that I saw something similar still being bitcasted on printf. Not sure if this should also get addressed as it is generally useful for implementations to know what variable a printf call refers to. But I think it's also simple to support in implementations and just look behind the cast.

; ModuleID = '<stdin>'
source_filename = "./test0.cac8cd49.b467a359.cl"
target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024"
target triple = "spir64-unknown-unknown"

@.str = private unnamed_addr addrspace(2) constant [5 x i8] c"%5d\0A\00", align 1

; Function Attrs: convergent nofree norecurse nounwind
define dso_local spir_kernel void @test0() local_unnamed_addr #0 !kernel_arg_addr_space !3 !kernel_arg_access_qual !3 !kernel_arg_type !3 !kernel_arg_base_type !3 !kernel_arg_type_qual !3 {
entry:
  %call = tail call spir_func i32 (ptr addrspace(2), ...) @printf(ptr addrspace(2) noundef @.str, i32 noundef 10) #2
  ret void
}

; Function Attrs: convergent nofree nounwind
declare spir_func noundef i32 @printf(ptr addrspace(2) nocapture noundef readonly, ...) local_unnamed_addr #1

attributes #0 = { convergent nofree norecurse nounwind "no-trapping-math"="true" "stack-protector-buffer-size"="8" "uniform-work-group-size"="false" }
attributes #1 = { convergent nofree nounwind "no-trapping-math"="true" "stack-protector-buffer-size"="8" }
attributes #2 = { convergent nounwind }

!llvm.module.flags = !{!0}
!opencl.ocl.version = !{!1}
!opencl.spir.version = !{!1}
!llvm.ident = !{!2}

!0 = !{i32 1, !"wchar_size", i32 4}
!1 = !{i32 3, i32 0}
!2 = !{!"clang version 16.0.5 (Fedora 16.0.5-1.fc38)"}
!3 = !{}

and that gets turned into something like this:

; SPIR-V
; Version: 1.0
; Generator: Khronos LLVM/SPIR-V Translator; 14
; Bound: 26
; Schema: 0
               OpCapability Addresses
               OpCapability Linkage
               OpCapability Kernel
               OpCapability Int64
               OpCapability Int8
          %1 = OpExtInstImport "OpenCL.std"
               OpMemoryModel Physical64 OpenCL
               OpEntryPoint Kernel %23 "test0"
               OpSource OpenCL_C 300000
               OpName %_str ".str"
               OpName %test0 "test0"
               OpDecorate %_str Constant
               OpDecorate %_str Alignment 1
               OpDecorate %test0 LinkageAttributes "test0" Export
      %uchar = OpTypeInt 8 0
      %ulong = OpTypeInt 64 0
       %uint = OpTypeInt 32 0
   %uchar_37 = OpConstant %uchar 37
   %uchar_53 = OpConstant %uchar 53
  %uchar_100 = OpConstant %uchar 100
   %uchar_10 = OpConstant %uchar 10
    %uchar_0 = OpConstant %uchar 0
    %ulong_5 = OpConstant %ulong 5
    %uint_10 = OpConstant %uint 10
%_arr_uchar_ulong_5 = OpTypeArray %uchar %ulong_5
%_ptr_UniformConstant__arr_uchar_ulong_5 = OpTypePointer UniformConstant %_arr_uchar_ulong_5
       %void = OpTypeVoid
         %15 = OpTypeFunction %void
%_ptr_UniformConstant_uchar = OpTypePointer UniformConstant %uchar
         %11 = OpConstantComposite %_arr_uchar_ulong_5 %uchar_37 %uchar_53 %uchar_100 %uchar_10 %uchar_0
       %_str = OpVariable %_ptr_UniformConstant__arr_uchar_ulong_5 UniformConstant %11
      %test0 = OpFunction %void None %15
         %17 = OpLabel
         %19 = OpBitcast %_ptr_UniformConstant_uchar %_str
         %22 = OpExtInst %uint %1 printf %19 %uint_10
               OpReturn
               OpFunctionEnd
         %23 = OpFunction %void None %15
         %24 = OpLabel
         %25 = OpFunctionCall %void %test0
               OpReturn
               OpFunctionEnd

@karolherbst
Copy link
Contributor

Same happens with the usage of event_t types.

@MrSidims
Copy link
Contributor

restarting CI

@MrSidims MrSidims closed this Jul 13, 2023
@MrSidims MrSidims reopened this Jul 13, 2023
@MrSidims MrSidims merged commit 322fca5 into KhronosGroup:llvm_release_160 Aug 4, 2023
8 of 9 checks passed
@mateuszchudyk mateuszchudyk deleted the i8-geps.backport16 branch March 21, 2024 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants