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

Generate GCC builtins for NVPTX target. #2556

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

redstar
Copy link
Member

@redstar redstar commented Feb 5, 2018

DCompute targets NVPTX, so we should generate the GCC builtins.

@thewilsonator
Copy link
Contributor

Nice.
Is there a way we can separate the intrinsics out into related clusters, like dcompute has now? e.g. send "llvm.nvvm.read.*" to index.d "llvm.nvvm.[barrier|membar]*" to sync.d. I think it would be much better w.r.t documentation and intuition of where things should be.

@thewilsonator
Copy link
Contributor

The generated file will have to be marked @compute(CompileFor.deviceOnly), with the corresponding import of ldc.dcompute.

@redstar
Copy link
Member Author

redstar commented Feb 5, 2018

Good hint! This requires an update to the gen_gccbuiltins tool, too.

Currently, the intrinsics are only generated. There is no clustering. Maybe we could just public import the intrinsics in the right module?

@thewilsonator
Copy link
Contributor

That works too.

@redstar
Copy link
Member Author

redstar commented Feb 5, 2018

Updated the gen_gccbuiltins with a small hack.

os << "; \n\nimport core.simd;\n\nnothrow @nogc:\n\n";
os << arch << "; \n\n";
if (isDcomputeOnly)
os << "ldc.dcompute;\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

You're missing an import that should precede that.

Copy link
Member

Choose a reason for hiding this comment

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

Could you add a testcase for this @redstar ? It would catch bugs like this.
(you can use // REQUIRES: target_NVPTX to filter)

DCompute targets NVPTX, so we should generate the GCC builtins.

The gen_gccbuiltins utility must be extended for this special case, too.
@kinke
Copy link
Member

kinke commented Feb 7, 2018

@JohanEngelen: tests/fuzz_basic.d finally failed again here; output:

3173: WARNING: Failed to find function "__sanitizer_print_stack_trace".
3173: WARNING: Failed to find function "__sanitizer_set_death_callback".
3173: INFO: Seed: 607937059
3173: INFO: Loaded 1 modules (17 guards): [0x46fc60, 0x46fca4), 
3173: INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes
3173: INFO: A corpus is not provided, starting from an empty corpus
3173: #0	READ units: 1
3173: #2	INITED cov: 12 ft: 7 corp: 1/1b exec/s: 0 rss: 23Mb
3173: #7	NEW    cov: 13 ft: 8 corp: 2/35b exec/s: 0 rss: 23Mb L: 34 MS: 5 ChangeByte-InsertByte-ShuffleBytes-ChangeBinInt-InsertRepeatedBytes-
3173: #1276	NEW    cov: 14 ft: 9 corp: 3/4131b exec/s: 0 rss: 23Mb L: 4096 MS: 4 ChangeByte-ShuffleBytes-CMP-CrossOver- DE: "\x08\x00\x00\x00\x00\x00\x00\x00"-
3173: #1048576	pulse  cov: 14 ft: 9 corp: 3/4131b exec/s: 524288 rss: 25Mb
3173: #1625455	NEW    cov: 15 ft: 10 corp: 4/8227b exec/s: 541818 rss: 25Mb L: 4096 MS: 3 ShuffleBytes-PersAutoDict-CMP- DE: "\x08\x00\x00\x00\x00\x00\x00\x00"-"U\x00\x00\x00"-
3173: uncaught exception
3173: core.exception.AssertError@/root/project/tests/sanitizers/fuzz_basic.d(18): Assertion failure
3173: ----------------
3173: ==15612== ERROR: libFuzzer: deadly signal
3173: NOTE: libFuzzer has rudimentary signal handlers.
3173:       Combine libFuzzer with AddressSanitizer or similar for better crash reports.
3173: SUMMARY: libFuzzer: deadly signal
3173: MS: 1 CMP- DE: "Z\x00"-; base unit: b5a0b0b13a0a6f20f09c6309e02984281ee78859
3173: artifact_prefix='./'; Test unit written to ./crash-13c73121ab89c2dde47584c826a38706c65d6b7f

@JohanEngelen
Copy link
Member

@thewilsonator Can you add a testcase for this? (// REQUIRES: target_NVPTX)

@thewilsonator
Copy link
Contributor

See #3993

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