Skip to content

Conversation

EricWF
Copy link
Member

@EricWF EricWF commented Jun 19, 2024

…e initial implementation

BitfieldSize(nullptr) {}
};

class ContractSpecifiers {
Copy link
Member Author

Choose a reason for hiding this comment

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

TODO: Remove this, I actually haven't used it yet.

Though I haven't fixed the parsing yet.

case Builtin::BI__builtin_operator_delete:
return HandleOperatorDeleteCall(Info, E);

case Builtin::BI__builtin_contract_assert: {
Copy link
Member Author

Choose a reason for hiding this comment

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

FIXME: This also is unused and needs to be removed.

ContractStmts need to be handled differently.

Builder.CreateCall(FnAssume, ArgValue);
return RValue::get(nullptr);
}
case Builtin::BI__builtin_contract_assert: {
Copy link
Member Author

Choose a reason for hiding this comment

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

Remove this as well.

Copy link

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

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

I think the next step would be to merge the 2 vectors of ContractStmts everywhere, it's really not ergonomic and one ought to be enough (it's should be easy to either sort it , or filter it)

]

_allStandards = ["c++03", "c++11", "c++14", "c++17", "c++20", "c++23", "c++26"]
_allStandards = ["c++03", "c++11", "c++14", "c++17", "c++20", "c++23", "c++26", "c++2c"]

Choose a reason for hiding this comment

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

What's the difference between 2c and 26?

enum { ResultNameDeclOffset = 0, ConditionOffset = 1, Count = 2 };

public:
// These types correspond to the three C++ 'await_suspend' return variants

Choose a reason for hiding this comment

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

haha :)

if (Tok.is(tok::identifier) && NextToken().is(tok::colon)) {
if (CK != ContractKind::Post) {
// Only post contracts can have a result name
Diag(Tok.getLocation(), diag::err_expected) << tok::identifier;

Choose a reason for hiding this comment

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

That diag looks weird

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it's wrong.

Actions.getASTContext().BoolTy);
}

T.consumeClose();

Choose a reason for hiding this comment

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

you want to keep that so that you know where the contract ends

Comment on lines 4389 to 4460
void Parser::ParsePostContract(Declarator &DeclaratorInfo) {
ConsumeToken();

ParseScope ParamScope(this, Scope::DeclScope |
Scope::FunctionDeclarationScope |
Scope::FunctionPrototypeScope);

DeclaratorChunk::FunctionTypeInfo FTI = DeclaratorInfo.getFunctionTypeInfo();
for (unsigned i = 0; i != FTI.NumParams; ++i) {
ParmVarDecl *Param = cast<ParmVarDecl>(FTI.Params[i].Param);
Actions.ActOnReenterCXXMethodParameter(getCurScope(), Param);
}

if (Tok.isNot(tok::l_paren)) {
Diag(Tok.getLocation(), diag::err_expected) << tok::l_paren;
return;
}
ConsumeParen();

// Post contracts start with <identifier> colon <expression>
// As we have to support the "auto f() post (r : r > 42) {...}" case, we cannot parse here
// the return type is not guaranteed to be known until after the function body parses

/*
if (Tok.isNot(tok::identifier)) {
Diag(Tok.getLocation(), diag::err_expected) << tok::identifier;
return;
}
ParsingDeclSpec DS(*this);
ParsedTemplateInfo TemplateInfo;
DeclSpecContext DSContext = getDeclSpecContextFromDeclaratorContext(DeclaratorContext::Block);
ParseDeclarationSpecifiers(DS, TemplateInfo, AS_none, DSContext);
ParsedAttributes LocalAttrs(AttrFactory);
ParsingDeclarator D(*this, DS, LocalAttrs, DeclaratorContext::Block);
D.setObjectType(getAsFunction().getReturnType());
IdentifierInfo *Id = Tok.getIdentifierInfo();
SourceLocation IdLoc = ConsumeToken();
D.setIdentifier(Id, IdLoc);
Decl* ThisDecl = Actions.ActOnDeclarator(getCurScope(), D);
Actions.ActOnUninitializedDecl(ThisDecl);
Actions.FinalizeDeclaration(ThisDecl);
D.complete(ThisDecl);
if (Tok.isNot(tok::colon)) {
Diag(Tok.getLocation(), diag::err_expected) << tok::colon;
return;
}
ExprResult Expr = ParseExpression();
if (Expr.isInvalid()) {
Diag(Tok.getLocation(), diag::err_invalid_pcs);
return;
}
DeclaratorInfo.addPostContract(Expr.get());
*/
ExprResult Expr = ParseExpression();
if (Expr.isInvalid()) {
Diag(Tok.getLocation(), diag::err_invalid_pcs);
return;
}
// DeclaratorInfo.addPostContract(Expr.get());

if (Tok.isNot(tok::r_paren)) {
Diag(Tok.getLocation(), diag::err_expected) << tok::r_paren;
return;
}
ConsumeParen();
}

Choose a reason for hiding this comment

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

Is that dead code?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, but I still might want to reference it while building the result name introducer decl later.

Comment on lines 2440 to 2442
// FIXME(EricWF): This seems really worg.
ParsedAttributes attrs(AttrFactory);
MaybeParseCXX11Attributes(attrs);

Choose a reason for hiding this comment

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

looks fine to me? (except we never do anything with these attributes)
We might want to make ContractStmt an AttributedStmt

Comment on lines 2585 to 2590
def ContractAssert : Builtin {
let Spellings = ["__builtin_contract_assert"];
let Attributes = [NoThrow, Constexpr];

let Prototype = "void(bool)";
}

Choose a reason for hiding this comment

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

dead code?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep.

Comment on lines 1678 to 1686

def fcontracts_impl_strategy : Joined<["-"], "fcontracts_strategy=">,
HelpText<"Specify contract implementation strategy. The default value is 'callee'.">,
Values<"0,1,2">,
Visibility<[ClangOption, CC1Option]>,
NormalizedValuesScope<"LangOptions::ContractImplStrategy">,
NormalizedValues<["Callee", "Both", "Split"]>,
MarshallingInfoEnum<LangOpts<"ContractStrategy">, "Callee">;

Choose a reason for hiding this comment

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

I like the ambition!

Copy link
Member Author

Choose a reason for hiding this comment

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

That's all peter :-) I'm going to only emit callee side.

Comment on lines +2110 to +2118
//===--------------------------------------------------------------------===//
// C++ Contracts
/*
ExceptionSpecificationType Parser::tryParseExceptionSpecification(
bool Delayed, SourceRange &SpecificationRange,
SmallVectorImpl<ParsedType> &DynamicExceptions,
SmallVectorImpl<SourceRange> &DynamicExceptionRanges,
ExprResult &NoexceptExpr, CachedTokens *&) {
*/

Choose a reason for hiding this comment

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

Weird comment

]

_allStandards = ["c++03", "c++11", "c++14", "c++17", "c++20", "c++23", "c++26"]
_allStandards = ["c++03", "c++11", "c++14", "c++17", "c++20", "c++23", "c++26", "c++2c"]

Choose a reason for hiding this comment

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

What is the difference between 26 and 2c?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nothing, I thought I needed it, but I don't.

@EricWF
Copy link
Member Author

EricWF commented Jun 21, 2024

I think the next step would be to merge the 2 vectors of ContractStmts everywhere, it's really not ergonomic and one ought to be enough (it's should be easy to either sort it , or filter it)

I think it's also needed to check that two declarations have equivalent contract specifications, since using two vectors removes the ordering of pre/post.

SourceLocation IdLoc = ConsumeToken();
(void)Id;
(void)IdLoc;
// FIXME(ericwf): Actually build the result name introducer
Copy link
Member Author

Choose a reason for hiding this comment

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

@cor3ntin I'm struggling to get the return type out of the parser here. Thoughts?

vporpo and others added 24 commits July 3, 2024 12:04
…on, OpaqueInst (llvm#97343)

A very basic implementation of sandboxir::
`Fuction`
`Argument`
`Constant`
`Instruction`
`OpaqueInst`
If the instruction is marked for deletion, better to drop all its
operands and mark them for deletion too (if allowed). It allows to have
more vectorizable patterns and generate less useless extractelement
instructions.

Reviewers: RKSimon

Reviewed By: RKSimon

Pull Request: llvm#97409
The new round-robin algorithm overrides the hash-based distribution of
functions to modules. It achieves a more even number of functions per
module when the number of functions is close to the number of requested
modules. It's not in use by default and is available under a new flag.
zext does not allow converting vector shadow to scalar, so we must
manually convert it prior to calling zext in materializeOneCheck, for
which the 'ConvertedShadow' parameter isn't actually guaranteed to be
scalar (1). Note that it is safe/no-op to call convertShadowToScalar on
a shadow that is already scalar.

In contrast, the storeOrigin function already converts the (potentially
vector) shadow to scalar; we add a comment to note why it is load
bearing.

(1) In materializeInstructionChecks():
"// Disable combining in some cases. TrackOrigins checks each shadow to
pick
 // correct origin.
 bool Combine = !MS.TrackOrigins;
 ...
       if (!Combine) {
        materializeOneCheck(IRB, ConvertedShadow, ShadowData.Origin);
        continue;
      }"
…s space when generating intrinsics (llvm#96836)

This PR aims to factor in the allocation address space provided by an
architectures data layout when generating the intrinsic instructions,
this allows them to be lowered later with the address spaces in tow.
This aligns the intrinsic creation with the LLVM IRBuilder's
https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/IR/IRBuilder.h#L1053

This is also necessary for the below example to compile for OpenMP AMD
GPU and not ICE the compiler in ISEL as AMD's stackrestore and stacksave
are expected to have the appropriate allocation address space for AMD
GPU.

program main
    integer(4), allocatable :: test
    allocate(test)

!$omp target map(tofrom:test)
    do i = 1, 10
      test = test + 50
    end do
!$omp end target

  deallocate(test)
end program

The PR also fixes the issue I opened a while ago which hits the same
error when compiling for AMDGPU:
llvm#82368

Although, you have to have the appropriate GPU LIBC and Fortran offload
runtime (both compiled for AMDGPU) added to the linker for the command
or it will reach another ISEL error and ICE weirdly. But with the
pre-requisites it works fine with this PR.
Added Demangle to Profile link components to fix shared build.
…lvm#97360)

llvm#95482 is a reland of
llvm#88024.
llvm#95482 keeps indexing memory
usage reasonable by using unordered_map and doesn't make other changes
to originally reviewed code.

While discussing possible ways to minimize indexing memory usage, Teresa
asked whether I need `ExportSetTy` as a map or a set is sufficient. This
PR implements the idea. It uses a set rather than a map to track exposed
ValueInfos.

Currently, `ExportLists` has two use cases, and neither needs to track a
ValueInfo's import/export status. So using a set is sufficient and
correct.
1) In both in-process and distributed ThinLTO, it's used to decide if a
function or global variable is visible [1] from another module after importing
creates additional cross-module references.
     * If a cross-module call edge is seen today, the callee must be visible
       to another module without keeping track of its export status already.
       For instance, this [2] is how callees of direct calls get exported.
2) For in-process ThinLTO [3], it's used to compute lto cache key.
     * The cache key computation already hashes [4] 'ImportList' , and 'ExportList' is
        determined by 'ImportList'. So it's fine to not track 'import type' for export list.

[1] https://github.com/llvm/llvm-project/blob/66cd8ec4c08252ebc73c82e4883a8da247ed146b/llvm/lib/LTO/LTO.cpp#L1815-L1819
[2] https://github.com/llvm/llvm-project/blob/66cd8ec4c08252ebc73c82e4883a8da247ed146b/llvm/lib/LTO/LTO.cpp#L1783-L1794
[3] https://github.com/llvm/llvm-project/blob/66cd8ec4c08252ebc73c82e4883a8da247ed146b/llvm/lib/LTO/LTO.cpp#L1494-L1496
[4] https://github.com/llvm/llvm-project/blob/b76100e220591fab2bf0a4917b216439f7aa4b09/llvm/lib/LTO/LTO.cpp#L194-L222
…U. NFC

We have an existing VT variable that should match N0.getValueType.
Add requires line to not test when the target architecture isn't
supported.

Technically we could make it a bit less restrictive, but want green
builds.
The two options are discussed in a few comments around
llvm#91280 (comment)

* -Wa,--crel: error "-Wa,--allow-experimental-crel must be specified to use -Wa,--crel..."
* -Wa,--allow-experimental-crel: no-op
* -Wa,--crel,--allow-experimental-crel: enable CREL in the integrated assembler (llvm#91280)

MIPS's little-endian n64 ABI messed up the `r_info` field in
relocations. While this could be fixed with CREL, my intention is to
avoid complication in assembler/linker. The implementation simply
doesn't allow CREL for MIPS.

Link: https://discourse.llvm.org/t/rfc-crel-a-compact-relocation-format-for-elf/77600

Pull Request: llvm#97378
- created integration tests for libc hdrgen 
- implemented sorting function names in yaml files through script
We only handled the easy LDS case before. Handle the other address
spaces
with the more complicated legality logic.
…#97329)

Reordered Function class parameter "standards" to make more logical
sense and to match the ordering in the add_function function.
Deleted the yaml_combined folder and moved contained files to the yaml
folder.
Updated math.yaml file with the recently added math functions in spec.td
Refactors legacy ranges writers to create a writer for each instance of
a DWO file.

We now write out everything into .debug_ranges after the all the DWO
files are processed. This also changes the order that ranges is written
out in, as before we wrote out while in the main CU processing loop and
we now iterate through the CU buckets created by partitionCUs, after the
main processing loop.
…7489)

Using the same range reduction as `sin`, `cos`, and `sincos`:
1) Reducing `x = k*pi/128 + u`, with `|u| <= pi/256`, and `u` is in
double-double.
2) Approximate `tan(u)` using degree-9 Taylor polynomial.
3) Compute
```
   tan(x) ~ (sin(k*pi/128) + tan(u) * cos(k*pi/128)) / (cos(k*pi/128) - tan(u) * sin(k*pi/128))
```
using the fast double-double division algorithm in [the CORE-MATH
project](https://gitlab.inria.fr/core-math/core-math/-/blob/master/src/binary64/tan/tan.c#L1855).
4) Perform relative-error Ziv's accuracy test
5) If the accuracy tests failed, we redo the computations using 128-bit
precision `DyadicFloat`.

Fixes llvm#96930
…nary operator& (llvm#97596)

Currently, `TreeTransform::TransformCXXOperatorCallExpr` calls
`TreeTransform::TransformAddressOfOperand` to transform the first
operand of a `CXXOperatorCallExpr` when its `OverloadOperatorKind` is
`OO_Amp` -- regardless of arity. This results in the first operand of
binary `operator&` being incorrectly transformed as if it was the
operand of the address of operator in cases such as the following:
```
struct A {
  int x;
};

void operator&(A, A);

template<typename T>
struct B {
  int f() {
    return T::x & 1; // invalid reference to 'A::x' is not diagnosed because 'T::x' is incorrectly transformed as if it was the operand of unary operator&
  }
};

template struct B<A>;
```
Prior to llvm#92318 we would build a `CXXDependentScopeMemberExpr` for
`T::x` (as with most dependent qualified names that were not member
qualified names). Since `TreeTransform::TransformAddressOfOperand` only
differs from `TransformExpr` for `DependentScopeDeclRefExpr` and
`UnresolvedLookupExpr` operands, `T::x` was transformed "correctly". Now
that we build a `DependentScopeDeclRefExpr` for `T::x`, it is
incorrectly transformed as if it was the operand of the address of
operator and we fail to diagnose the invalid reference to a non-static
data member. This patch fixes the issue by only calling
`TreeTransform::TransformAddressOfOperand` for `CXXOperatorCallExpr`s
with a single operand. This fixes llvm#97483.
The key was being dropped for external resources because they aren't
present in the dialect resource name mapper.
…n leading / trailing dimensions." (llvm#97652)

Reverts llvm#92934 because it breaks some lowering. To
repro: `mlir-opt -test-vector-transfer-flatten-patterns ~/repro.mlir`

```mlir
func.func @unit_dim_folding(%arg0: vector<1x1xf32>) -> vector<1x1xf32> {
  %cst = arith.constant dense<0.000000e+00> : vector<1x1xf32>
  %0 = arith.mulf %arg0, %cst : vector<1x1xf32>
  return %0 : vector<1x1xf32>
}
```
cor3ntin and others added 17 commits July 6, 2024 16:18
…lvm#97744)

GCC 14 has been released a while ago. We've updated the CI to use GCC 14
now. This removes any old annotations in the tests and updates the
documentation to reflect the updated version requirements.
…h Clang ToT targetting Windows"

This reverts commit 10e1b93.
…ang ToT targetting Windows"

This reverts commit 593f708.
This makes the test suite forward-compatible with future versions of macOS.
Previously, the Lit features were built in a way that they would assume
that any newer macOS version doesn't contain any version of LLVM, which
doesn't make sense.
The template stuff is a bit of a hack. I'm considering simply inserting
the statements into the function body and letting the existing code
handle the rest.

Also add some cheaky diagnostics, fix a few name lookup bugs, and
delete some old code.
There are also various experiements that need cleaning up,
specifically around builtins and source location.
Some work on constification, but a bunch of experiment that still needs
to be undone.

The addition of -fclang-contract-groups=foo=<semantic>, which still
needs better integrating with -fcontract-evaluation-semantic=.

The [[clang::contract_group("foo.bar")]] stuff works really well.
I'll be happy to try it in libc++.

What still needs to be done includes:
  * Coming up with a proper descriptor/format for the compiler-stl
    interface. It seems to me we'll want the ABI to be able to evolve
over time, and so we should pass a pointer to data + data descriptor of
some kind so that the STL can parse version of the header from different
compilers & versions.

  * Everything to do with parsing pre/post in inline member functions.
  * Contract redeclaration checking.
  * Constifying implicit this.
  * Code gen & constant evaluation for result names (and representation
    too).
  * Cleanup around experiments for source location & constification.
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.