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

wip -- emitFunctionProlog #1229

Draft
wants to merge 2,142 commits into
base: main
Choose a base branch
from
Draft

wip -- emitFunctionProlog #1229

wants to merge 2,142 commits into from

Conversation

lanza
Copy link
Member

@lanza lanza commented Dec 12, 2024

No description provided.

ghehg and others added 30 commits November 22, 2024 18:16
as title. 
The current implementation of this PR is use cir::CastOP integral
casting to implement vector type truncation. Thus, LLVM lowering code
has been change to accommodate it.
In addition.
Added code into
[CIRGenBuiltinAArch64.cpp](https://github.com/llvm/clangir/pull/909/files#diff-6f7700013aa60ed524eb6ddcbab90c4dd288c384f9434547b038357868334932)
to make it more similar to OG.
```
 mlir::Type ty = vTy;
  if (!ty)
```
Added test case into neon.c as the file already contains similar vector
move test cases such as vmovl

---------

Co-authored-by: Guojin He <guojinhe@meta.com>
as title. 
Also changed
[neon-ldst.c](https://github.com/llvm/clangir/compare/main...ghehg:clangir-llvm-ghehg:macM3?expand=1#diff-ea4814b6503bff2b7bc4afc6400565e6e89e5785bfcda587dc8401d8de5d3a22)
to make it have the same RUN options as OG
[clang/test/CodeGen/aarch64-neon-intrinsics.c](https://github.com/llvm/clangir/blob/main/clang/test/CodeGen/aarch64-neon-intrinsics.c)
Those options help us to avoid checking load/store pairs thus make the
test less verbose and easier to compare against OG.

Co-authored-by: Guojin He <guojinhe@meta.com>
Implement derived-to-base address conversions for non-virtual base
classes. The code gen for this situation was only implemented when the
offset was zero, and it simply created a `cir.base_class_addr` op for
which no lowering or other transformation existed.

Conversion to a virtual base class is not yet implemented.

Two new fields are added to the `cir.base_class_addr` operation: the
byte offset of the necessary adjustment, and a boolean flag indicating
whether the source operand may be null. The offset is easy to compute in
the front end while the entire path of intermediate classes is still
available. It would be difficult for the back end to recompute the
offset. So it is best to store it in the operation. The null-pointer
check is best done late in the lowering process. But whether or not the
null-pointer check is needed is only known by the front end; the back
end can't figure that out. So that flag needs to be stored in the
operation.

`CIRGenFunction::getAddressOfBaseClass` was largely rewritten. The code
path no longer matches the equivalent function in the LLVM IR code gen,
because the generated ClangIR is quite different from the generated LLVM
IR.

`cir.base_class_addr` is lowered to LLVM IR as a `getelementptr`
operation. If a null-pointer check is needed, then that is wrapped in a
`select` operation.

When generating code for a constructor or destructor, an incorrect
`cir.ptr_stride` op was used to convert the pointer to a base class. The
code was assuming that the operand of `cir.ptr_stride` was measured in
bytes; the operand is the number elements, not the number of bytes. So
the base class constructor was being called on the wrong chunk of
memory. Fix this by using a `cir.base_class_addr` op instead of
`cir.ptr_stride` in this scenario.

The use of `cir.ptr_stride` in `ApplyNonVirtualAndVirtualOffset` had the
same problem. Continue using `cir.ptr_stride` here, but temporarily
convert the pointer to type `char*` so the pointer is adjusted
correctly.

Adjust the expected results of three existing tests in response to these
changes.

Add two new tests, one code gen and one lowering, to cover the case
where a base class is at a non-zero offset.
Fix #934

While here move scope op codegen outside the builder, so it's easier
to dump blocks and operations while debugging.
After 5da4310, the LLVM dialect
requires the variadic callee type to be present for variadic calls. The
op builders take care of this automatically if you pass the function
type, so change our lowering logic to do so. Add tests for this as well
as a missing test for indirect function call lowering.

Fixes #913
Fixes #933
See the test for the reproducer. It would crash due the NYI.

See
https://github.com/llvm/llvm-project/blob/327124ece7d59de56ca0f9faa2cd82af68c011b9/clang/lib/CodeGen/CGExpr.cpp#L1295-L1373,
I found we've implemented all the cases in CGExpr.cpp. IIUC, I think we
can remove the NYI.
Close #883. See the above issue
for details
The title describes the purpose of the PR. 

The logic was gotten from the original CodeGen, and I added a test to
check that `-fno-PIE` is indeed enabled.
This PR adds support for the `__fp16` type. CIRGen and LLVM lowering is
included.

Resolve #900 .
…pare_and_swap (#955)

as title. 
Actually just follow the way in `makeBinaryAtomicValue` in the same file
which did the right thing by creating SInt or UInt based on first
argument's signess.
LLVM lowering support coming next.
…" (#944)

This reverts commit 8f699fd and fixes some issues, namely:
 
- CC lowering pass will no longer fail if the function has no AST
information that won't be used.
- Fixed CC lowering not disabling when running certain `cc1` compilation
commands.
- CC lowering can now be disabled when calling `cir-opt` and
`cir-translate`.
- Compilation commands that generate Object files should now invoke CC
lowering by default.
I tried to run llvm-test-suite and turned out that there are many tests
fail with segfault due to old C style (let's remember Kernighan and
Ritchie) . This PR fix it by the usual copy-pasta from the original
codegen :)

So let's take a look at the code:
```
void foo(x) short x; {}
int main() {
  foo(4); 
  return 0;
}
```
and CIR for `foo` function is:
```
cir.func  @foo(%arg0: !s32i) {
    %0 = cir.alloca !s16i, !cir.ptr<!s16i>, ["x", init] 
    %1 = cir.cast(bitcast, %0 : !cir.ptr<!s16i>), !cir.ptr<!s32i>
    cir.store %arg0, %1 : !s32i, !cir.ptr<!s32i>
    cir.return
}
```
We bitcast the **address** (!!!) and store a value of a bigger size
there.

And now everything looks fine:
```
cir.func no_proto  @foo(%arg0: !s32i) {
    %0 = cir.alloca !s16i, !cir.ptr<!s16i>, ["x", init] 
    %1 = cir.cast(integral, %arg0 : !s32i), !s16i
    cir.store %1, %0 : !s16i, !cir.ptr<!s16i> 
    cir.return 
} 
```
We truncate an argument and store it. 

P.S.
The `bitcast` that was there before looks a little bit suspicious and
dangerous. Are we sure we can do this unconditional cast while we create
`StoreOp` ?
This PR tries to give a simple initial implementation for eliminating
redundant loads of constant objects, an idea originally posted by
OfekShilon.

Specifically, this PR adds a new unit attribute `const` to the
`cir.alloca` operation. Presence of this attribute indicates that the
alloca-ed object is declared `const` in the input source program. CIRGen
is updated accordingly to start emitting this new attribute.
…nOp (#959)

They should use PoisonOp (which becomes PoisonValue in LLVMIR) as it is
the OG's choice.
Proof:
We generate VecCreateOp [here
](https://github.com/llvm/clangir/blob/2ca12fe5ec3a1e7279256f069010be2d68200585/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp#L1975)
And it's OG counterpart is
[here](https://github.com/llvm/clangir/blob/2ca12fe5ec3a1e7279256f069010be2d68200585/clang/lib/CodeGen/CGExprScalar.cpp#L2096)
OG uses PoisonValue. 
As to VecSplatOp, OG unconditionally [chooses PoisonValue
](https://github.com/llvm/clangir/blob/2ca12fe5ec3a1e7279256f069010be2d68200585/llvm/lib/IR/IRBuilder.cpp#L1204)

A even more solid proof for this case is that 
when we use OG to generate code for our test case I changed in this PR ,
its always using poison instead of undef as far as VecSplat and
VecCreate is concerned. The [OG generated code for vectype-ext.cpp
](https://godbolt.org/z/eqx1rns86) here.
The [OG generated code for vectype.cpp
](https://godbolt.org/z/frMjbKGeT) here.

For reference, generated CIR for the test case vectype-ext.cpp is
[here](https://godbolt.org/z/frMjbKGeT)

This is to unblock #936 to help it
set on the right path.

Note: There might be other CIR vec ops that need to choose Poison to be
consistent with OG, but I'd limit the scope of this PR, and wait to see
issue pop up in the future.
as title.
Base on my experience of [this type of test(https://github.com/llvm/clangir/blob/a7ac2b4e2055e169d9f556abf5821a1ccab666cd/clang/test/CIR/CodeGen/attribute-annotate-multiple.cpp#L51),
The number of characters varies in this line as it's about full file
path which changes during environment.
1. Add new `cir.vtt.address_point` op for visiting the element of VTT to
initialize the virtual pointer.
2. Implement `getVirtualBaseClassOffset` method which provides a virtual offset
to adjust to actual virtual pointers in virtual base.
3. Follows the original clang CodeGen scheme for the implementation of most
other parts.

@bcardosolopes's note: this is cherry-picked from an older PR from Jing Zhang and
slightly modified for updates: applied review, test, doc and operation syntax.
It does not yet has LLVM lowering support, I'm going to make incremental
changes on top of this.  Any necessary CIR modifications to this design should
follow up shortly too. Also, to make this work I also added more logic to
`addImplicitStructorParam`s` and `buildThisParam`.
as title. 
The generated code is the same as Clang codeden except in a small
discrepancy when GEP:
OG generates code like this: 
`%6 = getelementptr inbounds <4 x i16>, ptr %retval.i, i32 1`
CIR generates a bit differently:
`%6 = getelementptr <4 x i16>, ptr %retval.i, i64 1`
Ptr offest might be trivial because choosing i64 over i32 as index type
seems to be LLVM Dialect's choice.

The lack of `inbounds` keyword might be an issue as
`mlir::cir::PtrStrideOp` is currently not lowering to LLVM:GEPOp with
`inbounds` attribute as `mlir::cir::PtrStrideOp` itself has no
`inbounds`. It's probably because there was no need for it though we do
have an implementation of [`CIRGenFunction::buildCheckedInBoundsGEP`
](https://github.com/llvm/clangir/blob/10d6f4b94da7e0181a070f0265d079419d96cf78/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp#L2762).

Anyway, the issue is not in the scope of this PR and should be addressed
in a separate PR. If we think this is an issue, I can create another PR
and probably add optional attribute to `mlir::cir::PtrStrideOp` to
achieve it.

In addition to lowering work, a couple of more works:

1. Did a little refactoring on variable name changing into desired
CamelBack case.
2. Changed neon-misc RUN Options to be consistent with other neon test
files and make test case more concise.
as title. 
There are two highlights of the PR

1. The PR introduced a new test file to cover neon intrinsics that move
data, which is a big category. This would the 5th neon test file. And
we're committed to keep total number of neon test files within 6. This
file uses another opt option instcombine, which makes test LLVM code
more concise, and our -fclangir generated LLVM code would be identical
to OG with this. It looks like OG did some instcombine optimization.

2. `getIntFromMLIRValue` helper function could be substituted by
[`mlir::cir::IntAttr getConstOpIntAttr` in
CIRGenAtomic.cpp](https://github.com/llvm/clangir/blob/24b24557c98d1c031572a567b658cfb6254f8a89/clang/lib/CIR/CodeGen/CIRGenAtomic.cpp#L337).
The function `mlir::cir::IntAttr getConstOpIntAttr` is doing more than
`getIntFromMLIRValue`, and there is FIXME in the comment, so not sure if
we should just use `mlir::cir::IntAttr getConstOpIntAttr`, either is
fine with me.
…ly (#961)

Close #957

the previous algorithm to convert a multiple dimension array to a tensor
is: fill the value one by one and fill the zero values in conditions.
And it has some problems handling the multiple dimension array as above
issue shows so that the generated values are not in the same shape with
the original array.

the new algorithm here is, full fill the values ahead of time with the
correct element size and full fill the values to different slots and we
only need to maintain the index to write.

I feel the new version has better performance (avoid allocation) and
better readability slightly.
ghehg and others added 27 commits November 27, 2024 15:02
[OG's implementation
](https://github.com/llvm/clangir/blob/aaf38b30d31251f3411790820c5e1bf914393ddc/clang/lib/CodeGen/CGBuiltin.cpp#L7527)
provides one common code to handle all neon SISD intrinsics. But IMHO,
it entangles different things together which hurts readability.
Here, We start with simple easy-to-understand approach with specific
case. And in the future, as we handle more intrinsics, we may come up
with a few simple common patterns.
Co-authored-by: Sirui Mu <msrlancern@gmail.com>
This PR adds `clang::CodeGenOptions` to the lowering context. Similar to
`clang::LangOptions`, the code generation options are currently set to
the default values when initializing the lowering context.

Besides, this PR also adds a new attribute `#cir.opt_level`. The
attribute is a module-level attribute and it holds the optimization
level (e.g. -O1, -Oz, etc.). The attribute is consumed when initializing
the lowering context to populate the `OptimizationLevel` and the
`OptimizeSize` field in the code generation options. CIRGen is updated
to attach this attribute to the module op.
Removes some NYIs. But left assert(false) due to missing tests. It looks
better since it is not so scaring as NYI.
This PR adds support for base-to-derived and derived-to-base casts on
pointer-to-data-member values.

Related to #973.
#1194)

Basically, for int type, the order of Ops is not the same as OG in the
emitted LLVM IR. OG has constant as the second op position. See [OG's
order ](https://godbolt.org/z/584jrWeYn).
Default assignment operator generation was failing because of memcpy
generation for fields being unsupported. Implement it following
CodeGen's example, as usual. Follow-ups will avoid emitting memcpys for
fields of trivial class types, and extend this to copy constructors as
well.

Fixes #1128
Our previous logic here was matching CodeGen, which folds trivial
assignment operator calls into memcpys, but we want to avoid that. Note
that we still end up emitting memcpys for arrays of classes with trivial
assignment operators; #1177 tracks
fixing that.
CodeGen does so for trivial record types as well as non-record types; we
only do it for non-record types.
This is a leftover from when ClangIR was initially focused on analysis
and could ignore default method generation. We now handle default
methods and should generate them in all cases. This fixes several bugs:
- Default methods weren't emitted when emitting LLVM, only CIR.
- Default methods only referenced by other default methods weren't
  emitted.
This PR updates the LLVM lowering part of load and stores to const
allocas and makes use of the !invariant.group metadata in the result
LLVM IR.

The HoistAlloca pass is also updated. The const flag on a hoisted alloca
is removed for now since their uses are not always invariants. Will
update in later PRs to teach their invariants.
This PR adds CIRGen support for C++23 `[[assume(expr)]]` statement.
There's a lot that was missing here. Add NYIs and MissingFeatures as
appropriate, so that we can start work on matching CodeGen.
Revert "[CIR][Dialect] Introduce StdInitializerListOp to represent
high-level semantics of C++ initializer list (#1121)".

This reverts commit 7532e05.

First step for #1215.
ClangIR CodeGen makes use of both `mlir::MLIRContext` and
`clang::ASTContext`. Referring to these as just "context" can be
ambiguous.

This change attempts to make the CodeGen code more clear by choosing
better variable names. The change is almost entirely renaming: mostly
change variables, parameters, and fields named `context` or `ctx` to
`mlirContext` or `astContext`. There are some other renames having to do
with contexts, and a small number of other drive-by fixes. (I considered
also renaming functions named `getContext()`, but did not include that
in this change.)

This is entirely code cleanup.  There should be no behavior changes.
This can't be simply implemented by our CIR Add via LLVM::AddOp, as
i[t's saturated add.](https://godbolt.org/z/MxqGrj6fP)
…ch OG

We are missing cleanups all around, more incremental progress towards fixing
that. This is supposed to be NFC intended, but we have to start changing some
bits in order to properly match cleanup bits in OG.

Start tagging places with more MissingFeatures to allow us to incrementally
improve the situation.
…n to match OG"

Seems like windows bots are now broken!

This reverts commit 9a63c50.
Created using spr 1.3.5
Copy link

github-actions bot commented Dec 12, 2024

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 2b1a638ea07ca10c5727ea835bfbe17b881175cc 0ad87afcb32c2777af86f6b629e1306aa883b653 --extensions cpp,h -- clang/test/CIR/idk.cpp clang/include/clang/CIR/MissingFeatures.h clang/lib/CIR/CodeGen/CIRGenCall.cpp clang/lib/CIR/CodeGen/CIRGenDecl.cpp clang/lib/CIR/CodeGen/CIRGenFunction.cpp clang/lib/CIR/CodeGen/CIRGenFunction.h
View the diff from clang-format here.
diff --git a/clang/lib/CIR/CodeGen/CIRGenDecl.cpp b/clang/lib/CIR/CodeGen/CIRGenDecl.cpp
index 708a87a138..cd978481a6 100644
--- a/clang/lib/CIR/CodeGen/CIRGenDecl.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenDecl.cpp
@@ -1267,7 +1267,7 @@ void CIRGenFunction::pushDestroyAndDeferDeactivation(
 /// Emit an alloca (or GlobalValue depending on target)
 /// for the specified parameter and set up LocalDeclMap.
 void CIRGenFunction::emitParmDecl(const VarDecl &varDecl, ParamValue arg,
-                                   unsigned argNo) {
+                                  unsigned argNo) {
   bool noDebugInfo = false;
   // FIXME: Why isn't ImplicitParamDecl a ParmVarDecl?
   assert((isa<ParmVarDecl>(varDecl) || isa<ImplicitParamDecl>(varDecl)) &&
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h
index e0a7c58a0e..0beb0b9333 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.h
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h
@@ -975,12 +975,11 @@ public:
 
   void emitInvariantStart(CharUnits Size);
 
-
   /// emitFunctionProlog - Emit the target specific CIR code to load the
   /// arguments for the given function. This is also responsible for naming the
   /// MLIR function arguments.
   void emitFunctionProlog(const CIRGenFunctionInfo &functionInfo,
-                           cir::FuncOp fn, const FunctionArgList &args);
+                          cir::FuncOp fn, const FunctionArgList &args);
 
   /// Create a check for a function parameter that may potentially be
   /// declared as non-null.

Created using spr 1.3.5
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.