Use demangled name for BuiltinWithGenericPointer

[bmanga]

This tries to address #1343

@rjodinchr should I also add a test?

[google-cla[bot]]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[rjodinchr]

should I also add a test?

It would be great to add the simple example you gave in the issue. Maybe just running clang front-end as in https://github.com/google/clspv/blob/main/test/emit_ir.cl#L1

[bmanga]

If I add the --output-format=ll to the test file then the error is not triggered as I think it happens at a later stage. Keeping the test file as I had it originally gives (for the original code):

FAIL: clspv :: BuiltinsWithGenericPointer/issue-1343.cl (1 of 12)
******************** TEST 'clspv :: BuiltinsWithGenericPointer/issue-1343.cl' FAILED ********************
Exit Code: 255

Command Output (stdout):
--
# RUN: at line 1
clspv --cl-std=CLC++ --inline-entry-points  /home/bruno/Projects/clspv/test/BuiltinsWithGenericPointer/issue-1343.cl -o /home/bruno/Projects/clspv/build/test/BuiltinsWithGenericPointer/Output/issue-1343.cl.tmp.spv
# executed command: clspv --cl-std=CLC++ --inline-entry-points /home/bruno/Projects/clspv/test/BuiltinsWithGenericPointer/issue-1343.cl -o /home/bruno/Projects/clspv/build/test/BuiltinsWithGenericPointer/Output/issue-1343.cl.tmp.spv
# .---command stderr------------
# | /home/bruno/Projects/clspv/test/BuiltinsWithGenericPointer/issue-1343.cl:4:7: warning: no previous prototype for function 'contains_frexp_in_name'
# |     4 | float contains_frexp_in_name(float, int *) {}
# |       |       ^
# | /home/bruno/Projects/clspv/test/BuiltinsWithGenericPointer/issue-1343.cl:4:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
# |     4 | float contains_frexp_in_name(float, int *) {}
# |       | ^
# |       | static 
# | /home/bruno/Projects/clspv/test/BuiltinsWithGenericPointer/issue-1343.cl:4:45: warning: non-void function does not return a value
# |     4 | float contains_frexp_in_name(float, int *) {}
# |       |                                             ^
# | /home/bruno/Projects/clspv/test/BuiltinsWithGenericPointer/issue-1343.cl:6:6: warning: no previous prototype for function 'vstore_half5'
# |     6 | void vstore_half5(float2, size_t, half *) {}
# |       |      ^
# | /home/bruno/Projects/clspv/test/BuiltinsWithGenericPointer/issue-1343.cl:6:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
# |     6 | void vstore_half5(float2, size_t, half *) {}
# |       | ^
# |       | static 
# | target datalayout = "e-p:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-G1"; Function Attrs: convergent mustprogress norecurse nounwind
# | define dso_local spir_func float @_Z22contains_frexp_in_namefPi(float %0, ptr %1)
# | ; Function Attrs: convergent mustprogress norecurse nounwind
# | define dso_local spir_func float @_Z22contains_frexp_in_namefPU3AS1i(float %0, ptr addrspace(1) %1)
# | ; Function Attrs: convergent mustprogress norecurse nounwind
# | define dso_local spir_func float @_Z22contains_frexp_in_namefPU3AS3i(float %0, ptr addrspace(3) %1)
# | @clspv.builtins.used = appending global [3 x ptr] [ptr @_Z22contains_frexp_in_namefPi, ptr @_Z22contains_frexp_in_namefPU3AS1i, ptr @_Z22contains_frexp_in_namefPU3AS3i], section "llvm.metadata"
# | internal_additional_library:: error: expected '{' in function body
# | define dso_local spir_func float @_Z22contains_frexp_in_namefPU3AS1i(float %0, ptr addrspace(1) %1)
# | ^
# `-----------------------------
# error: command failed with exit status: 255

--

And passes with the PR changes.

I think it should be good enough?

[rjodinchr]

We don't like adding a test without explicit checks. It makes it quite complicated sometimes when it starts failing for whatever reason to know what the test was expecting to check.

As it's the link that is failing and it is performed after the GenerateIRFile, it makes sense that it does not work with --output-format. What we can do instead is to use --print-before=annotation-to-metadata to get the ll file after the link. So that we can check that the content is what we expect and nothing more.

[bmanga]

Mh I've tried that and it seems like clspv crashes before being able to dump anything to the file...

[rjodinchr]

Without the fix, it should crash before anything is dumped. Am I missing something?

[bmanga]

Ah ok I misunderstood your previous comment. I thought you wanted the test to fail through FileCheck and I assumed that flag would output something I could act upon without crashing the compiler.

Updated. Let me know if that's ok :)