- In D33414, if any function call was found within a kernel, we would bail out.
- This is an over-approximation. This patch changes this by allowing intrinsics within kernels.
- This introduces an additional step when creating a separate llvm::Module for a kernel. We now have to copy the intrinsics from the original llvm::Module to the kernel-specific module.
- Note that this maybe too lax. Perhaps we are allowing intrinsics which should not be allowed through. This can be debated in the review.
efriedma jdoerfert Meinersbur gareevroman sebpop • zinob huihuiz pollydev grosser singam-sanjay
- rGf291c8d510d2: [PPCGCodeGeneration] Allow intrinsics within kernels.
rPLO306284: [PPCGCodeGeneration] Allow intrinsics within kernels.
rL306284: [PPCGCodeGeneration] Allow intrinsics within kernels.
@bollu The patch did prevent passing the address of the debug intrinsic to the kernel, but couldn't guard against buggy metadata,
DICompileUnit not listed in llvm.dbg.cu !11 = distinct !DICompileUnit(language: DW_LANG_C89, file: !3, producer: "julia", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !12)
I'll have to look closer at how *GPUModule is being produced to understand this error.
@bollu Please refer this comment on my patch. I observed a pattern with functions that produced the buggy metadata, simple copy kernels (A[i][j][k] = B[i][j][k]) didn't produce buggy metadata but kernels that had computation (A[i][j][k] +=/-=/*= B[i][j][k]) resulted in Modules that didn't list the DICompileUnit in llvm.dbg.cu.
I'm not sure how a module that's built independent from source IR carries metadata. Do you think there'd be a function to explicitly list the DICompileUnit in llvm.dbg.cu ?
I believe the Julia problem consists of two (partially independent) bugs. If this patch fixes a part of your problem (the intrinsics), then we should likely commit it and thing about the metadata afterwards.
- [NFC] typo fix.
- Try to retreive pre-existing function. If this does not exist, call Function::Create.
- [NFC] add a debug message if we are bailing out from PPCGCodeGeneration due to an unrecognised function in a kernel.
- Changed approach as discussed offline with Tobias. It was decided to use the BlockGenerator functionality to setup correct kernel code generation, rather than fixing generated code.
- General patch cleanup.
Some more comments.
|1120 ↗||(On Diff #102326)|
In llvm we start function names with lowercase letters.
Also, please document what the parameter F does.
Also, the comment is not useful. Please describe in one sentence what the function does, and then in a small paragraph what the criteria which a function that is listed here must fullfill.
|1122 ↗||(On Diff #102326)|
Please make this an explicit list of intrinsics. Maybe start with sqrt only to extablish the infrastructure.
|2704 ↗||(On Diff #102326)|
Also, in general we comment functions as follow:
/// Brief one-line comment /// /// Longer paragraph explaining what is happening here.
|2816 ↗||(On Diff #102326)|
Do not add braces.
|401 ↗||(On Diff #103923)|
Create new function DECLARATIONS ...
|408 ↗||(On Diff #103923)|
Should be IslNodeBuilder::ValueMap
|1136 ↗||(On Diff #103923)|
Start with a comment describing the function.
|1138 ↗||(On Diff #103923)|
|1122 ↗||(On Diff #102326)|
Please add a comment that we match any llvm.sqrt intrinsics by doing a string match.