This is an archive of the discontinued LLVM Phabricator instance.

Intrinsics: Add type overload to stacksave and stackstore
ClosedPublic

Authored by arsenm on Jul 31 2023, 4:08 AM.

Details

Summary

This allows use with non-0 address space stacks. llvm_ptr_ty should
never be used. This could use some more percolation up through mlir,
but this is enough to fix existing tests.

Diff Detail

Event Timeline

arsenm created this revision.Jul 31 2023, 4:08 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 31 2023, 4:08 AM
arsenm requested review of this revision.Jul 31 2023, 4:08 AM
Herald added a project: Restricted Project. · View Herald Transcript
arsenm updated this revision to Diff 545596.Jul 31 2023, 4:50 AM

Update documentation

This seems generally reasonable.

clang/lib/CodeGen/CGBuiltin.cpp
3910

Why not CreateStackSave? (same question for CGDecl.)

clang/lib/CodeGen/CGDecl.cpp
1631

Does this CreateTempAlloca call need to be updated?

arsenm updated this revision to Diff 545844.Jul 31 2023, 4:02 PM
arsenm marked an inline comment as done.

Fix temp slot type, oversized and overaligned for AMDGPU since it used the default address space pointer but wasn't broken.

Also assert types match and use the Create* functions

arsenm added inline comments.Jul 31 2023, 4:06 PM
clang/lib/CodeGen/CGBuiltin.cpp
3910

I was considering possible multiple stack handling or something, where you'd want to use the current instance's type. It shouldn't matter in reality

nikic added a subscriber: nikic.Aug 9 2023, 7:17 AM

There is an assertion failure in llvmir-intrinsics.mlir.

arsenm updated this revision to Diff 548728.Aug 9 2023, 12:06 PM
arsenm edited the summary of this revision. (Show Details)
arsenm added a reviewer: krzysz00.
Herald added a reviewer: aartbik. · View Herald Transcript
Herald added a reviewer: ftynse. · View Herald Transcript
Herald added a reviewer: dcaballe. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
nikic accepted this revision.Aug 9 2023, 12:17 PM

LGTM, though I did not review test changes in detail.

This revision is now accepted and ready to land.Aug 9 2023, 12:17 PM