Skip to content

Arm64_32 #1

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

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Arm64_32 #1

wants to merge 16 commits into from

Conversation

TNorthover
Copy link
Owner

Just a dummy pull request to make the patch sequence a bit more obvious in GitHub. The arm64_32 branch is what's interesting.

@@ -51,7 +51,11 @@ AArch64TargetInfo::AArch64TargetInfo(const llvm::Triple &Triple,
HasLegalHalfType = true;
HasFloat16 = true;

LongWidth = LongAlign = PointerWidth = PointerAlign = 64;
if (!Triple.getArchName().endswith("_32"))
Copy link

Choose a reason for hiding this comment

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

You use this predicate a lot; is there a better way of expressing it now?

@@ -261,6 +261,7 @@ Triple::ArchType Triple::getArchTypeForLLVMName(StringRef Name) {
.Case("aarch64_be", aarch64_be)
.Case("arc", arc)
.Case("arm64", aarch64) // "arm64" is an alias for "aarch64"
.Case("arm64_32", aarch64) // "arm64" is an alias for "aarch64"
Copy link

Choose a reason for hiding this comment

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

Why does this not have its own arch? That seems extremely bug-prone for all clients of the triple.

Copy link
Owner Author

Choose a reason for hiding this comment

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

That sounds like a really good idea, but it didn't occur to me at the time.

Originally I favoured aarch64-apple-watchos-code32, but that caused problems with LTO and the binutilsy tools which only wanted to look at the architecture part of the triple.

A new arch here shouldn't suffer from that, though we'll probably need to update lldb & ld64 (and Swift of course).

I'll start a prototype.

Copy link

Choose a reason for hiding this comment

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

Thanks! I assume that arch will also be considered 32-bit?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yep, I implemented it today and a large part of the benefit IMO has been that isArch32Bit is now available straight from the Triple for when you want to check for ILP32.

Copy link

Choose a reason for hiding this comment

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

Awesome! That'll knock out a number of special cases we've had to track down and add in various places.

TNorthover pushed a commit that referenced this pull request Feb 6, 2019
…implify (#1)

This is the second of a series of patches simplifying llvm-symbolizer
tests. See r352752 for the first. This one splits out 5 distinct test
cases from llvm-symbolizer.test into separate tests, and simplifies them
slightly by using --obj/positional arguments for the input file and
addresses instead of stdin.

See https://bugs.llvm.org/show_bug.cgi?id=40070#c1 for the motivation.

Reviewed by: dblaikie

Differential Revision: https://reviews.llvm.org/D57443

llvm-svn: 352753
TNorthover pushed a commit that referenced this pull request Jan 27, 2021
… disabled for a PCH vs a module file

This addresses an issue with how the PCH preable works, specifically:

1. When using a PCH/preamble the module hash changes and a different cache directory is used
2. When the preamble is used, PCH & PCM validation is disabled.

Due to combination of #1 and llvm#2, reparsing with preamble enabled can end up loading a stale module file before a header change and using it without updating it because validation is disabled and it doesn’t check that the header has changed and the module file is out-of-date.

rdar://72611253

Differential Revision: https://reviews.llvm.org/D95159
TNorthover pushed a commit that referenced this pull request Mar 26, 2024
…lvm#85653)

This reverts commit daebe5c.

This commit causes the following asan issue:

```
<snip>/llvm-project/build/bin/mlir-opt <snip>/llvm-project/mlir/test/Dialect/XeGPU/XeGPUOps.mlir | <snip>/llvm-project/build/bin/FileCheck <snip>/llvm-project/mlir/test/Dialect/XeGPU/XeGPUOps.mlir
# executed command: <snip>/llvm-project/build/bin/mlir-opt <snip>/llvm-project/mlir/test/Dialect/XeGPU/XeGPUOps.mlir
# .---command stderr------------
# | =================================================================
# | ==2772558==ERROR: AddressSanitizer: stack-use-after-return on address 0x7fd2c2c42b90 at pc 0x55e406d54614 bp 0x7ffc810e4070 sp 0x7ffc810e4068
# | READ of size 8 at 0x7fd2c2c42b90 thread T0
# |     #0 0x55e406d54613 in operator()<long int const*> /usr/include/c++/13/bits/predefined_ops.h:318
# |     #1 0x55e406d54613 in __count_if<long int const*, __gnu_cxx::__ops::_Iter_pred<mlir::verifyListOfOperandsOrIntegers(Operation*, llvm::StringRef, unsigned int, llvm::ArrayRef<long int>, ValueRange)::<lambda(int64_t)> > > /usr/include/c++/13/bits/stl_algobase.h:2125
# |     llvm#2 0x55e406d54613 in count_if<long int const*, mlir::verifyListOfOperandsOrIntegers(Operation*, 
...
```
TNorthover pushed a commit that referenced this pull request Mar 26, 2024
…oint. (llvm#83821)"

This reverts commit c2c1e6e. It creates
a use after free.

==8342==ERROR: AddressSanitizer: heap-use-after-free on address 0x50f000001760 at pc 0x55b9fb84a8fb bp 0x7ffc18468a10 sp 0x7ffc18468a08
READ of size 1 at 0x50f000001760 thread T0
 #0 0x55b9fb84a8fa in dropPoisonGeneratingFlags llvm/lib/Transforms/Vectorize/VPlan.h:1040:13
 #1 0x55b9fb84a8fa in llvm::VPlanTransforms::dropPoisonGeneratingRecipes(llvm::VPlan&, llvm::function_ref<bool (llvm::BasicBlock*)>)::$_0::operator()(llvm::VPRecipeBase*) const llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:1236:23
 llvm#2 0x55b9fb84a196 in llvm::VPlanTransforms::dropPoisonGeneratingRecipes(llvm::VPlan&, llvm::function_ref<bool (llvm::BasicBlock*)>) llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp

Can be reproduced with asan on
Transforms/LoopVectorize/AArch64/sve-interleaved-masked-accesses.ll
Transforms/LoopVectorize/X86/pr81872.ll
Transforms/LoopVectorize/X86/x86-interleaved-accesses-masked-group.ll
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.

2 participants