[all-commits] [llvm/llvm-project] a4cfb1: [Sema] Sanity-check alignment requested via `__att...
Roman Lebedev via All-commits
all-commits at lists.llvm.org
Thu Jan 23 11:53:20 PST 2020
Branch: refs/heads/master
Home: https://github.com/llvm/llvm-project
Commit: a4cfb15d15a8a353fe316f2a9fe1c8c6a6740ef1
https://github.com/llvm/llvm-project/commit/a4cfb15d15a8a353fe316f2a9fe1c8c6a6740ef1
Author: Roman Lebedev <lebedev.ri at gmail.com>
Date: 2020-01-23 (Thu, 23 Jan 2020)
Changed paths:
M clang/lib/Sema/SemaDeclAttr.cpp
M clang/test/Sema/builtin-assume-aligned.c
Log Message:
-----------
[Sema] Sanity-check alignment requested via `__attribute__((assume_aligned(imm)))`
Summary:
For `__builtin_assume_aligned()`, we do validate that the alignment
is not greater than `536870912` (D68824), but we don't do that for
`__attribute__((assume_aligned(N)))` attribute.
I suspect we should.
Reviewers: erichkeane, aaron.ballman, hfinkel, rsmith, jdoerfert
Reviewed By: erichkeane
Subscribers: cfe-commits, llvm-commits
Tags: #llvm, #clang
Differential Revision: https://reviews.llvm.org/D72994
Commit: c2a9061ac5166e48fe85ea2b6dbce9457c964958
https://github.com/llvm/llvm-project/commit/c2a9061ac5166e48fe85ea2b6dbce9457c964958
Author: Roman Lebedev <lebedev.ri at gmail.com>
Date: 2020-01-23 (Thu, 23 Jan 2020)
Changed paths:
M clang/lib/Sema/SemaChecking.cpp
M clang/test/Sema/alloc-align-attr.c
M clang/test/SemaCXX/alloc-align-attr.cpp
Log Message:
-----------
[Sema] Attempt to perform call-size-specific `__attribute__((alloc_align(param_idx)))` validation
Summary:
`alloc_align` attribute takes parameter number, not the alignment itself,
so given **just** the attribute/function declaration we can't do any
sanity checking for said alignment.
However, at call site, given the actual `Expr` that is passed
into that parameter, we //might// be able to evaluate said `Expr`
as Integer Constant Expression, and perform the sanity checks.
But since there is no requirement for that argument to be an immediate,
we may fail, and that's okay.
However if we did evaluate, we should enforce the same constraints
as with `__builtin_assume_aligned()`/`__attribute__((assume_aligned(imm)))`:
said alignment is a power of two, and is not greater than our magic threshold
Reviewers: erichkeane, aaron.ballman, hfinkel, rsmith, jdoerfert
Reviewed By: erichkeane
Subscribers: cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D72996
Commit: d096f8d306b2b16a25f65ffb70849ca7963a0dac
https://github.com/llvm/llvm-project/commit/d096f8d306b2b16a25f65ffb70849ca7963a0dac
Author: Roman Lebedev <lebedev.ri at gmail.com>
Date: 2020-01-23 (Thu, 23 Jan 2020)
Changed paths:
M clang/include/clang/Sema/Sema.h
M clang/lib/Sema/SemaChecking.cpp
M clang/lib/Sema/SemaDeclAttr.cpp
M llvm/lib/IR/Attributes.cpp
Log Message:
-----------
[IR] Attribute/AttrBuilder: use Value::MaximumAlignment magic constant
Summary:
I initially encountered those assertions when trying to create
this IR `alignment` attribute from clang's `__attribute__((assume_aligned(imm)))`,
because until D72994 there is no sanity checking for the value of `imm`.
But even then, we have `llvm::Value::MaximumAlignment` constant (which is `536870912`),
which is enforced for clang attributes, and then there are some other magical constant
(`0x40000000` i.e. `1073741824` i.e. `2 * 536870912`) in
`Attribute::getWithAlignment()`/`AttrBuilder::addAlignmentAttr()`.
I strongly suspect that `0x40000000` is incorrect,
and that also should be `llvm::Value::MaximumAlignment`.
Reviewers: erichkeane, hfinkel, jdoerfert, gchatelet, courbet
Reviewed By: erichkeane
Subscribers: hiraditya, cfe-commits, llvm-commits
Tags: #llvm, #clang
Differential Revision: https://reviews.llvm.org/D72998
Commit: e819f7c9feb4d4b611681e319fbb43fd28b3f5b7
https://github.com/llvm/llvm-project/commit/e819f7c9feb4d4b611681e319fbb43fd28b3f5b7
Author: Roman Lebedev <lebedev.ri at gmail.com>
Date: 2020-01-23 (Thu, 23 Jan 2020)
Changed paths:
M clang/lib/CodeGen/CGCall.cpp
M clang/test/CodeGen/assume-aligned-and-alloc-align-attributes.c
M clang/test/CodeGen/builtin-assume-aligned.c
M clang/test/CodeGen/catch-alignment-assumption-attribute-assume_aligned-on-function.cpp
Log Message:
-----------
[Codegen] If reasonable, materialize clang's `AssumeAlignedAttr` as llvm's Alignment Attribute on call-site function return value
Summary:
This should be mostly NFC - we still lower the same alignment
knowledge to the IR. The main reasoning here is that
this somewhat improves readability of IR like this,
and will improve test coverage in upcoming patch.
Even though the alignment is guaranteed to always be an I-C-E,
we don't always materialize it as llvm's Alignment Attribute because:
1. There may be a non-zero offset
2. We may be sanitizing for alignment
Note that if there already was an IR alignment attribute
on return value, we union them, and thus the alignment
only ever rises.
Also, there is a second relevant clang attribute `AllocAlignAttr`,
so that is why `AbstractAssumeAlignedAttrEmitter` is templated.
Reviewers: erichkeane, jdoerfert, hfinkel, aaron.ballman, rsmith
Reviewed By: erichkeane
Subscribers: cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D73005
Commit: 5ffe6408ffb6ad8642598ab03de04d58b4980e81
https://github.com/llvm/llvm-project/commit/5ffe6408ffb6ad8642598ab03de04d58b4980e81
Author: Roman Lebedev <lebedev.ri at gmail.com>
Date: 2020-01-23 (Thu, 23 Jan 2020)
Changed paths:
M clang/lib/CodeGen/CGCall.cpp
M clang/test/CodeGen/assume-aligned-and-alloc-align-attributes.c
M clang/test/CodeGen/catch-alignment-assumption-attribute-alloc_align-on-function.cpp
Log Message:
-----------
[Codegen] If reasonable, materialize clang's `AllocAlignAttr` as llvm's Alignment Attribute on call-site function return value
Summary:
Much like with the previous patch (D73005) with `AssumeAlignedAttr`
handling, results in mildly more readable IR,
and will improve test coverage in upcoming patch.
Note that in `AllocAlignAttr`'s case, there is no requirement
for that alignment parameter to end up being an I-C-E.
Reviewers: erichkeane, jdoerfert, hfinkel, aaron.ballman, rsmith
Reviewed By: erichkeane
Subscribers: cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D73006
Commit: b749af6a1ff45434011278c3ba765a41f0ee02f2
https://github.com/llvm/llvm-project/commit/b749af6a1ff45434011278c3ba765a41f0ee02f2
Author: Roman Lebedev <lebedev.ri at gmail.com>
Date: 2020-01-23 (Thu, 23 Jan 2020)
Changed paths:
M clang/lib/Sema/SemaDeclAttr.cpp
A clang/test/SemaCXX/std-align-val-t-in-operator-new.cpp
Log Message:
-----------
[Sema] Don't disallow placing `__attribute__((alloc_align(param_idx)))` on `std::align_val_t`-typed parameters
Summary:
I kind-of understand why it is restricted to integer-typed arguments,
for general enum's the value passed is not nessesairly the alignment implied,
although one might say that user would know best.
But we clearly should whitelist `std::align_val_t`,
which is just a thin wrapper over `std::size_t`,
and is the C++ standard way of specifying alignment.
Reviewers: erichkeane, rsmith, aaron.ballman, jdoerfert
Reviewed By: erichkeane
Subscribers: cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D73019
Compare: https://github.com/llvm/llvm-project/compare/9ad044a38c00...b749af6a1ff4
More information about the All-commits
mailing list