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

[CIR][CodeGen][NFC] Add getMLIRContext to CIRGenModule #1047

Merged
merged 1,996 commits into from
Nov 5, 2024

Conversation

lanza
Copy link
Member

@lanza lanza commented Nov 1, 2024

Upstream review of a PR requested that we be more explicit with
differentiating things from MLIR to similarly named things from clang
AST/LLVM/etc. So add an MLIRContext getter that we should start using.

ghehg and others added 30 commits October 12, 2024 00:29
… of std::initializer_list (#764)

implement VisitCXXStdInitializerListExpr as similar as to
[OG](https://github.com/llvm/clangir/blob/7150a050c12119c27e9eb1547aa65f535e4bfbe9/clang/lib/CodeGen/CGExprAgg.cpp#L417):
In order to support this implementation, made some changes to get more
helper functions.
Also added some tests.

The generated LLVM code is most similar to OG's llvm code, 3 interesting
differences:
1.  CIR introduced scope, thus extra branch
2. OG' has comdat for _ZSt1fIiEvSt16initializer_listIT_E function, but
we haven't implemented FuncOP's comdat yet. I'll probably try to add it
in another PR, but it's not in the scope of this PR.
3. When defining initialized_list type, OG prefers generic type like
%"class.std::initializer_list" = type { ptr, ptr }, but CIR prefers
instantiated like "%"class.std::initializer_list<const char *>" = type {
ptr, ptr }"
This PR adds CIRGen and LLVMIR codegen for those not-yet-covered complex
casts, including explicit type cast expressions of complex types and
complex value promotion.

All type conversion expressions involving complex types should now
codegen.
This PR verifies `cir.get_global` has its result type correctly
annotated with address space of the referenced symbol. The documentation
is also updated to clarify this constraint.

`GlobalOp` is the main consideration. It's worth noting that if the
`cir.get_global` op references a function, we also (implicitly) checks
that its result pointer type has no address space attribute.
This PR sets proper address space when creating `cir.global` and
`cir.get_global`.

Different languages use different ways to encode the address space in
AST constructs (i.e. VarDecl *). OpenCL and SYCL use an address space
qualifier on the type of `VarDecl`, while CUDA uses separate AST
attributes like `CUDASharedAttr`. Similarily, some targets may want to
use special address space for global variables. So a per-language +
per-target hook is needed to provide this customization point. In the
LLVM CodeGen, it's the helper method `getGlobalVarAddressSpace` that
takes on the role.

For OpenCL C + SPIR-V combination, OpenCL C converts the address space
qualifier to corresponding LangAS, but SPIR-V does not require any
action.

This PR implements `global` qualifier in OpenCL C, but does not include
`constant` qualifier. Although the modified part works for `constant`,
CIRGen is not yet able to set constant attribute for global ops (there
is a TODO line).

Static variable decl and `local` qualifier work in a similar way and
come in later patches.
Unary decrement expression on floating point operands was lowered to
`fsub -1.0` by a typo. This PR fixes this bug.
…otected (#776)

This PR add a new CIR attribute `mlir::cir::VisibilityAttr` to represent
CIR visibility. It will represent C/C++ visibility type `Default`,
`Hidden`, `Protected`.

The PR handles the parsing, printing of CIR visibility and also lower to
LLVM.

After this PR, there will be more PR's to migrate CIRGen properties that
are currently querying MLIR visibility(e.g. `sym_visibility`), to
instead query CIR visibility, and remove MLIR's visibility from printing
and parsing.
This PR adds CIRGen and LLVMIR lowering for unary increment and
decrement expressions of complex types.

Currently blocked by #789 .
…d variables (#792)

In OpenCL, `local`-qualified variables are implicitly considered as
static. In order to support it, this PR unblocks code paths related to
OpenCL static declarations in `CIRGenDecl.cpp`.

Following the approach of LLVM CodeGen, a new class
`CIRGenOpenCLRuntime` is added to handle the language hook of creating
`local`-qualified variables. The default behavior of this hook is quite
simple. It forwards the call to `CGF.buildStaticVarDecl`.

So in CIR, the OpenCL local memory representation is equivalent to the
one defined by SPIR-LLVM convention: a `cir.global` with
`addrspace(local)` and *without initializer*, which corresponds to LLVM
`undef` initializer. See check lines in test for more details.

A `static global`-qualified variable is also added in the test to
exercise the static code path itself.
This patch fixes a bunch of pending review comments in #784:

 - Remove data layout attribute from address space testing
 - Remove incoherent comment
 - Rename abi_or_pref to abiOrPref
 - Make comments impersonal
 - Implement feature guard for ARM's CMSE secure call feature
 - Track volatile return times feature in CC lowering
 - Track missing features in the Itanium record builder
 - Remove incoherent fix me
 - Clarify comment regarding CIR record layout getter
 - Track missing cache for record layout getter
 - Remove unnecessary todo's
This PR adds the initial CIRGen support for pointer-to-member-functions.
It contains the following new types, attributes, and operations:

  - `!cir.method`, which represents the pointer-to-member-function type.
- `#cir.method`, which represents a literal pointer-to-member-function
value that points to ~~non-virtual~~ member functions.
- ~~`#cir.virtual_method`, which represents a literal
pointer-to-member-function value that points to virtual member
functions.~~
- ~~`cir.get_method_callee`~~ `cir.get_method`, which resolves a
pointer-to-member-function to a function pointer as the callee.

See the new test at `clang/test/CIR/CIRGen/pointer-to-member-func.cpp`
for how these new CIR stuff works to support
pointer-to-member-functions.
This PR adds a new `cir.select` operation. This operation won't be
generated directly by CIRGen but it is useful during further CIR to CIR
transformations.

This PR addresses #785 .
There is an implementation for inline variables processing at CIR. The
LIT test was taken from clang's cxx1z-inline-variables.cpp where the
same functionality is tested for Clang Code generation. The test can be
run as follows
```
bin/llvm-lit -v ../clang/test/CIR/CodeGen/cxx1z-inline-variables.cpp
```

Note: the pull request also contains a formatting change for two files:
- `clang/lib/CIR/Dialect/IR/CIRDataLayout.cpp`
- `clang/lib/CIR/Dialect/Transforms/TargetLowering/Targets/X86.cpp`
mixed signed and unsigned integer operator cause difference result when
index of array is negative
…800)

This PR refactors the LoweringPrepare pass and replaces various ternary
ops generated by LoweringPrepare with semantically equivalent select
ops.
#799)

`__sync_fetch_and_add` currently doesn't support unsigned integers. 

The following code snippet, for example, raises an error:  
```
#include <stdint.h>

void foo(uint64_t x) {
  __sync_fetch_and_add(&x, 1);
}
```
The error can be traced down to this line `auto intType =
builder.getSIntNTy(cgf.getContext().getTypeSize(typ));` from
`clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp`.
…ry fp2fp operations (#806)

This PR is the continuation and refinement of PR #434 which is
originally authored by @philnik777 . Does not update it in-place since I
don't have commit access to Nikolas' repo.

This PR basically just rebases #434 onto the latest `main`. I also
updated some naming used in the original PR to keep naming styles
consistent.

Co-authored-by: Nikolas Klauser <[email protected]>
Still missing CFG flattening and lowering, coming next.
lanza and others added 23 commits October 29, 2024 21:54
Reviewers: smeenai

Reviewed By: smeenai

Pull Request: #1022
We diverge from CodeGen here by delaying the function emission that
happens for a global variable. However, due to situations where a global
can be emitted while building out a function the old CGF might not be
invalid. So we need to store it here just in case.

Reviewers: bcardosolopes, smeenai

Reviewed By: smeenai

Pull Request: #1023
This was declared but never implemented. Upon first usage in a later
commit this fails to link.

Reviewers: bcardosolopes, smeenai

Reviewed By: smeenai

Pull Request: #1024
…tion lowering pass (#1003)

This PR adds initial function pointers support for the calling
convention lowering pass. This is a suggestion, so any other ideas are
welcome.

Several ideas was described in the #995 and basically what I'm trying to
do is to generate a clean CIR code without additional `bitcast`
operations for function pointers and without mix of lowered and initial
function types.

#### Problem
Looks like we can not just lower the function type and cast the value
since too many operations are involved. For instance, for the
next simple code: 
```
typedef struct {
  int a;
} S;

typedef int (*myfptr)(S);

int foo(S s) { return 42 + s.a; }

void bar() {
  myfptr a = foo;
}
```
we get the next CIR for the function `bar` , before the calling
convention lowering pass:
```
cir.func no_proto  @bar() extra(#fn_attr) {
    %0 = cir.alloca !cir.ptr<!cir.func<!s32i (!ty_S)>>, !cir.ptr<!cir.ptr<!cir.func<!s32i (!ty_S)>>>, ["a", init]
    %1 = cir.get_global @foo : !cir.ptr<!cir.func<!s32i (!ty_S)>> 
    cir.store %1, %0 : !cir.ptr<!cir.func<!s32i (!ty_S)>>, !cir.ptr<!cir.ptr<!cir.func<!s32i (!ty_S)>>>
    cir.return 
  } 
```
As one can see, first three operations depend on the function type. Once
`foo` is lowered, we need to fix `GetGlobalOp`:
otherwise the code will fail with the verification error since actual
`foo` type (lowered) differs from the one currently expected by the
`GetGlobalOp`.
First idea would just rewrite only the `GetGlobalOp` and insert a
bitcast after, so both `AllocaOp` and `StoreOp` would work witth proper
types.

Once the code will be more complex, we will need to take care about
possible use cases, e.g. if we use arrays, we will need to track array
accesses to it as well in order to insert this bitcast every time the
array element is needed.

One workaround I can think of: we fix the `GetGlobalOp` type and cast
from the lowered type to the initial, and cast back before the actual
call happens - but it doesn't sound as a good and clean approach (from
my point of view, of course).

So I suggest to use type converter and rewrite any operation that may
deal with function pointers and make sure it has a proper type, and we
don't have any unlowered function type in the program after the calling
convention lowering pass.

#### Implementation
I added lowering for `AllocaOp`, `GetGlobalOp`, and split the lowering
for `FuncOp` (former `CallConvLoweringPattern`) and lower `CallOp`
separately.
Frankly speaking, I tried to implement a pattern for each operation, but
for some reasons the tests are not passed for
windows and macOs in this case - something weird happens inside
`applyPatternsAndFold` function. I suspect it's due to two different
rewriters used - one in the `LoweringModule` and one in the mentioned
function.
So I decided to follow the same approach as it's done for the
`LoweringPrepare` pass and don't involve this complex rewriting
framework.

Next I will add a type converter for the struct type, patterns for
`ConstantOp` (for const arrays and `GlobalViewAttr`)
In the end of the day we'll have (at least I hope so) a clean CIR code
without any bitcasts for function pointers.

cc @sitio-couto  @bcardosolopes
We had some incorrect logic when creating functions and getting their
address which resulted in spurious "definition with the same mangled
name" errors. Fix that logic to match original CodeGen, which also fixes
these errors.

It's expected that decls can appear in the deferred decl list multiple
times, and CodeGen has to guard against that. In the case that triggered
the error, both `CIRGenerator::HandleInlineFunctionDefinition` and
CIRGenModule were deferring the declaration.

Something else I discovered here is that we emit these functions in the
opposite order as regular CodeGen: https://godbolt.org/z/4PrKG7h9b.
That might be a meaningful difference worth investigating further.

Fixes #991
…intrinsics (#1020)

In this PR, also changed `buildNeonShiftVector` to allow it generates
negative shift values. When the shift value is negative, the shift
amount vector is not used in any ShiftOp of IR (as they don't need sign
to know shift direction), instead, it is just input argument to shift
intrinsic function call.
)

This PR fixes the notorious double whitespaces introduced by visibility
attribute, for `cir.func` only.

It uses "leading whitespace" for every print. And the printing of
visibility attr is properly guarded by a check of `!isDefault()`.

Double whitespaces in test files are removed.
This patch introduces support for the abs family of built-in functions
(abs, labs, llabs).
Add lowering prepare logic to lower stores to cir.copy.

This bring LLVM lowering closer to OG and turns out the rest of the compiler
understands memcpys better and generate better assembly code for at least arm64
and x86_64.

Note that current lowering to memcpy is only using i32 intrinsic version,
this PR does not touch that code and that will be addressed in another PR.
Clang recognizes the function anyway, but this is an obvious error.
In addition, this PR enables ZeroAttr of vector type so that CIR can
generate a vector initialized with all zero values.
due to the issue described in
#1018,
the MLIR lowering for `memmove` has been excluded in this patch.
…bits (#1027)

This PR adds a support for the return values of struct types > 128 bits
in size.
As usually, lot's of copy-pasted lines from the original codegen, except
the `AllocaOp` replacement for the return value.
… convention lowering pass (#1034)

This PR adds a support for calls by  function pointers.

@sitio-couto   I think would be great if you'll also take a look
This PR adds calling convention lowering support for the int128 type on
x86_64.

This is a follow up on #953 .
Created using spr 1.3.5
@lanza lanza requested a review from bcardosolopes as a code owner November 1, 2024 20:06
Copy link
Member

@bcardosolopes bcardosolopes left a comment

Choose a reason for hiding this comment

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

LGTM

smeenai and others added 2 commits November 5, 2024 13:42
Created using spr 1.3.5

[skip ci]
Created using spr 1.3.5
@lanza lanza changed the base branch from spr/lanza/main.circodegennfc-add-getmlircontext-to-cirgenmodule to main November 5, 2024 18:43
@lanza lanza merged commit 9e2ff80 into main Nov 5, 2024
7 of 8 checks passed
@lanza lanza deleted the spr/lanza/circodegennfc-add-getmlircontext-to-cirgenmodule branch November 5, 2024 18:43
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.