[CIR][CodeGen] Support for _mm_prefetch .#1522
[CIR][CodeGen] Support for _mm_prefetch .#1522shrikardongre wants to merge 0 commit intollvm:mainfrom
Conversation
| return nullptr; | ||
| case X86::BI_mm_prefetch: { | ||
| llvm_unreachable("_mm_prefetch NYI"); | ||
| mlir::Location loc = mlir ::UnknownLoc::get(&getMLIRContext()); |
There was a problem hiding this comment.
The location is known here: you can use getLoc(E->getExprLoc()) to retrieve it.
| auto C = dyn_cast<cir::IntAttr>( | ||
| cast<cir::ConstantOp>(Ops[1].getDefiningOp()).getValue()); | ||
| auto Loc = mlir::IntegerAttr::get( | ||
| mlir::IntegerType::get(builder.getContext(), 64), C.getUInt()); |
There was a problem hiding this comment.
You probably forgot an & 0x3 after C.getUInt(), and the integer type should be 32-bit rather than 64.
See OG:
Value *Locality = ConstantInt::get(Int32Ty, C->getZExtValue() & 0x3);There was a problem hiding this comment.
Oh yes , I was just trying something and didn't remove it from the commit. Thanks.
| mlir::UnitAttr Write = | ||
| ReadWrite ? mlir::UnitAttr::get(&getMLIRContext()) : nullptr; | ||
| return builder.create<cir::PrefetchOp>(loc, Ops[0], Loc, Write) | ||
| ->getResult(0); |
There was a problem hiding this comment.
cir::PrefetchOp expects a void*, but Ops[0] may be some other pointer type or an array type. You need to add builder.createBitcast(...) to convert between them.
Moreover, the PrefetchOp doesn't have a result, so ->getResult(0) will raise an error at runtime. But we can't return a nullptr or {} either, because the emitBuiltinExpr function will think these return values mean "unimplemented". Not sure what to return.
There was a problem hiding this comment.
We have a cir::CastOp will implement using that 👍🏻
There was a problem hiding this comment.
@AdUhTkJm @bcardosolopes I am finding it a little hard to implement the CastOp by just looking at the tablegen definition and the implementation given at https://llvm.github.io/clangir/Dialect/ops.html#circast-circastop
def CastOp : CIR_Op<"cast",
[Pure,
DeclareOpInterfaceMethods<PromotableOpInterface>]> {
operation ::= `cir.cast` `(` $kind `,` $src `:` type($src) `)`
`,` type($result) attr-dict
I am not able to find any implementation can you suggest some source ??
There was a problem hiding this comment.
Also I had a doubt , the Ops[0] doesnt seem to be a pointer type
llvm::SmallVector<mlir::Value, 4U> Ops
There was a problem hiding this comment.
You can look at CIRGenBuilder.h and CIRGenBaseBuilder.h to find some useful functions. For example, to create a bitcast, obtain a builder and write builder.createBitcast(...).
The website doesn't give a full implementation of ops. Try looking at CIROps.td. To see what the fields mean, Operation Definition Specification on MLIR website will be helpful. You might also refer to TableGen programmer's reference for TableGen syntax.
Ops[0] is an mlir::Value; itself isn't a type, but it has some type (you can get it with getType()). In PrefetchOp the value is required to have a void pointer type.
There was a problem hiding this comment.
Thanks ,will look into it 👍
this is what I was able to write by taking the reference of the in clang/lib/CIR/CodeGen/CIRGenBuilder.h |
|
the Current implementation is this is without a return type using a PrefetchOp now , and finally this was the first time i did not get a stack dump :) the test file is : But the generated CIR seems to be incorrect :( (if i am not wrong). |
I believe there is
Without an We still need to return a |
|
Yes there are several occurances of Anyways just making the file PR open for review to seek for advice . |
|
Thanks for the review @AdUhTkJm
This is orthogonal to the cast feedback |
|
@bcardosolopes can you please explain what did you mean by the above comment also I am not able to generate the correct CIR as I tried again today , do i need to implement it using the |
|
@shrikardongre I think you can ignore my comment, not relevant to fix this PR, it was a mere note.
You should emit the same LLVM as OG codegen does and we prefer to reuse existing operations if possible. Effectively, this means that if |
35a21ca to
6502f55
Compare
This draft PR needs review
I have to confirm if the implementation in the clang/lib/CIR/CodeGen/CIRGenBuiltinX86.cpp is correct.
also to add a proper FileCheck instruction in the clang/test/CIR/CodeGen/X86/builtins-x86.c .