Skip to content

Conversation

steffenlarsen
Copy link
Contributor

Because exported device functions are interspersed with kernels in the offload entries they are thought to be kernels in device images. These changes use the list of exported symbols generated by sycl-post-link to filter out the exported device functions when creating device images.

Because exported device functions are interspersed with kernels in the
offload entries they are thought to be kernels in device images. These
changes use the list of exported symbols generated by sycl-post-link to
filter out the exported device functions when creating device images.

Signed-off-by: Steffen Larsen <[email protected]>
@steffenlarsen
Copy link
Contributor Author

Regression test: intel/llvm-test-suite#486

Signed-off-by: Steffen Larsen <[email protected]>
DeviceBinaryImage::PropertyRange DeviceLibReqMask;
DeviceBinaryImage::PropertyRange KernelParamOptInfo;
DeviceBinaryImage::PropertyRange ProgramMetadata;
DeviceBinaryImage::PropertyRange ExportedSymbols;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smaslov-intel do we have usages of this structure outside of sycl/source directory? If not, we should move this class inside the lib, otherwise this is a breaking change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I see something similar is done for getSpecConstantsDefaultValues. I will adopt that approach.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has been changed. It should no longer be a breaking change. Thanks for pointing it out!

@dm-vodopyanov dm-vodopyanov merged commit 4112cbc into intel:sycl Sep 30, 2021
alexbatashev added a commit to alexbatashev/llvm that referenced this pull request Oct 3, 2021
* sycl: (108 commits)
  [SYCL][XPTI] Revisit resource management strategy (intel#4494)
  [SYCL][ESIMD] Fix misprint: ESIMD_L1_FLUASH_RO_DATA -> ESIMD_L1_FLUSH_RO_DATA (intel#4681)
  [SYCL] Make kernel_bundle interop more conformant (intel#4672)
  [SYCL] Submission with kernel parameter ignores set kernel bundle (intel#4667)
  [SYCL] Add support for std::byte to vec class  (intel#4637)
  [BuildBot] Uplift CPU/FPGAEMU RT version for CI Process (intel#4671)
  [SYCL] Fix an error on host when big image is used on opencl:gpu (intel#4668)
  [SYCL] Exclude exported symbols from kernel bundles (intel#4660)
  Revert "[SYCL] Allow overriding plugin libraries (intel#4067)" (intel#4659)
  [SYCL] Handle exceptions on mutually exclusive handler operations (intel#4639)
  [sycl-post-link] Don't split module if function pointer has a user that's not CallInst (intel#4657)
  [SYCL][HIP] Fix MemBufferFill for nvidia platform (intel#4629)
  [SYCL][Doc] Describe DPC++ CUDA install w/ non-standard toolkit loc (intel#4663)
  [SYCL] Fix device code instrumentation (intel#4615)
  Remove myself as a code owner (intel#4653)
  [SYCL] Fix overwriting insert to sub_group_mask (intel#4656)
  [x86][Matrix] Replace packed_a with row_major in matrix testcases (intel#4641)
  [SYCL][Doc] Add device global extension spec (intel#4233)
  [sycl-post-link] Adds property listing exported functions (intel#4626)
  [Driver][SYCL] Do not consider non-archive files for FPGA binary checks (intel#4644)
  ...
alexbatashev added a commit to alexbatashev/llvm that referenced this pull request Oct 3, 2021
* sycl: (107 commits)
  [SYCL][XPTI] Revisit resource management strategy (intel#4494)
  [SYCL][ESIMD] Fix misprint: ESIMD_L1_FLUASH_RO_DATA -> ESIMD_L1_FLUSH_RO_DATA (intel#4681)
  [SYCL] Make kernel_bundle interop more conformant (intel#4672)
  [SYCL] Submission with kernel parameter ignores set kernel bundle (intel#4667)
  [SYCL] Add support for std::byte to vec class  (intel#4637)
  [BuildBot] Uplift CPU/FPGAEMU RT version for CI Process (intel#4671)
  [SYCL] Fix an error on host when big image is used on opencl:gpu (intel#4668)
  [SYCL] Exclude exported symbols from kernel bundles (intel#4660)
  Revert "[SYCL] Allow overriding plugin libraries (intel#4067)" (intel#4659)
  [SYCL] Handle exceptions on mutually exclusive handler operations (intel#4639)
  [sycl-post-link] Don't split module if function pointer has a user that's not CallInst (intel#4657)
  [SYCL][HIP] Fix MemBufferFill for nvidia platform (intel#4629)
  [SYCL][Doc] Describe DPC++ CUDA install w/ non-standard toolkit loc (intel#4663)
  [SYCL] Fix device code instrumentation (intel#4615)
  Remove myself as a code owner (intel#4653)
  [SYCL] Fix overwriting insert to sub_group_mask (intel#4656)
  [x86][Matrix] Replace packed_a with row_major in matrix testcases (intel#4641)
  [SYCL][Doc] Add device global extension spec (intel#4233)
  [sycl-post-link] Adds property listing exported functions (intel#4626)
  [Driver][SYCL] Do not consider non-archive files for FPGA binary checks (intel#4644)
  ...
@steffenlarsen steffenlarsen deleted the steffen/kernel_bundle_ignore_device_funcs branch December 6, 2023 11:37
iclsrc pushed a commit that referenced this pull request Aug 12, 2025
M68k's SETCC instruction (`scc`) distinctly fills the destination byte
with all 1s. If boolean contents are set to `ZeroOrOneBooleanContent`,
LLVM can mistakenly think the destination holds `0x01` instead of `0xff`
and emit broken code as a result. This change corrects the boolean
content type to `ZeroOrNegativeOneBooleanContent`.

For example, this IR:

```llvm
define dso_local signext range(i8 0, 2) i8 @testBool(i32 noundef %a) local_unnamed_addr #0 {
entry:
  %cmp = icmp eq i32 %a, 4660
  %. = zext i1 %cmp to i8
  ret i8 %.
}
```

would previously build as:

```asm
testBool:                               ; @testBool
	cmpi.l	#4660, (4,%sp)
	seq	%d0
	and.l	#255, %d0
	rts
```

Notice the `zext` is erroneously not clearing the low bits, and thus the
register returns with 255 instead of 1. This patch fixes the issue:

```asm
testBool:                               ; @testBool
	cmpi.l	#4660, (4,%sp)
	seq	%d0
	and.l	#1, %d0
	rts
```

Most of the tests containing `scc` suffered from the same value error as
described above, so those tests have been updated to match the new
output (which also logically corrects them).
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.

4 participants