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

Translator doesn't handle LLVM integer types. #481

Open
bader opened this issue Mar 31, 2020 · 9 comments
Open

Translator doesn't handle LLVM integer types. #481

bader opened this issue Mar 31, 2020 · 9 comments
Labels
agenda Tooling Call Agenda bug Something isn't working

Comments

@bader
Copy link
Contributor

bader commented Mar 31, 2020

https://godbolt.org/z/p3D7TX - legit OpenCL code, which translator can't convert to SPIR-V due to i3 type.
The translator with the assertions gives me:

InvalidBitWidth: Invalid bit width in input: 3

Proper LLVM back-end should be able to handle such types...

Is there an easy way to avoid such types in LLVM IR by configuring SPIR target in clang?

@bader bader added bug Something isn't working agenda Tooling Call Agenda labels Mar 31, 2020
@AlexeySachkov
Copy link
Contributor

Error for this LLVM IR is expected: from this comment:

2.16.1. Universal Validation Rules
Data rules
Scalar integer types can be parameterized only as 32 bit, plus any additional sizes enabled by capabilities.

However, I agree that input source code is pretty valid and shouldn't cause any compilation errors somewhere in the compiler stack

Note: support to arbitrary precision integers is on the way into the translator via support of corresponding extension, see #480

@bader
Copy link
Contributor Author

bader commented Mar 31, 2020

The validation rule applies to SPIR-V, not LLVM IR.
Translator is trying to do 1:1 mapping for integer type width, which leads to the rule violation.
Alternative approach is map LLVM integer types to "legal" SPIR-V types. This is what LLVM back-ends are doing when they lower LLVM IR to HW ISA.

@keryell
Copy link
Member

keryell commented Mar 31, 2020

Interesting code which happens to be naturally FPGA friendly! :-)
I am curious about how it is lowered to FPGA efficiently then?

@svenvh
Copy link
Member

svenvh commented Apr 1, 2020

The validation rule applies to SPIR-V, not LLVM IR.
Translator is trying to do 1:1 mapping for integer type width, which leads to the rule violation.
Alternative approach is map LLVM integer types to "legal" SPIR-V types. This is what LLVM back-ends are doing when they lower LLVM IR to HW ISA.

Agreed, the translator is only doing a best-effort mapping to SPIR-V currently. We should consider adding a legalization pass to the translator to deal with cases like these.

@StuartDBrady
Copy link
Contributor

Note that i1 is currently mapped to bool: any legalization would need to avoid breaking bool, but should still properly legalize any use of i1 as a single-bit integer by frontends.

@bader
Copy link
Contributor Author

bader commented Apr 7, 2020

I've found "an easy way to avoid such types in LLVM IR by configuring SPIR target in clang".

diff --git a/clang/lib/Basic/Targets/SPIR.h b/clang/lib/Basic/Targets/SPIR.h
index b24d0107d51..40a740a76b1 100644
--- a/clang/lib/Basic/Targets/SPIR.h
+++ b/clang/lib/Basic/Targets/SPIR.h
@@ -136,7 +136,7 @@ public:
     SizeType = TargetInfo::UnsignedInt;
     PtrDiffType = IntPtrType = TargetInfo::SignedInt;
     resetDataLayout("e-p:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-"
-                    "v96:128-v192:256-v256:256-v512:512-v1024:1024");
+                    "v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:48");
   }
 
   void getTargetDefines(const LangOptions &Opts,
@@ -152,7 +152,7 @@ public:
     PtrDiffType = IntPtrType = TargetInfo::SignedLong;
 
     resetDataLayout("e-i64:64-v16:16-v24:32-v32:32-v48:64-"
-                    "v96:128-v192:256-v256:256-v512:512-v1024:1024");
+                    "v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:48");
   }
 
   void getTargetDefines(const LangOptions &Opts,

Unfortunately it doesn't solve all the problems with integer type widths.
There are still cases when optimizations are using 48-bit constants to initialize <3 x i16> vectors. I don't have an OpenCL reproducer for this case yet. I'll try to create one later.

@bader
Copy link
Contributor Author

bader commented Apr 7, 2020

Oops... I see the typo in data layout string, which might explain the reason why 48-bit constants are used by LLVM transformations. I'll check if it's the reason.

@MrSidims
Copy link
Contributor

@bader can we consider SPV_INTEL_arbitrary_precision_integers to be good enough to solve the issue or you would prefer an approach, that doesn't require any extensions?

@bader
Copy link
Contributor Author

bader commented Sep 28, 2022

Original OpenCL code doesn't require extensions, so I expect the same from SPIR-V format of the program.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agenda Tooling Call Agenda bug Something isn't working
Projects
None yet
Development

No branches or pull requests

7 participants