Skip to content

Commit 9a26876

Browse files
committed
address reviewer comments
1 parent a789891 commit 9a26876

File tree

8 files changed

+59
-79
lines changed

8 files changed

+59
-79
lines changed

mlir/include/mlir/Dialect/Ptr/IR/PtrAttrDefs.td

+11-17
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
include "mlir/Dialect/Ptr/IR/PtrDialect.td"
1313
include "mlir/Dialect/Ptr/IR/MemorySpaceInterfaces.td"
1414
include "mlir/IR/AttrTypeBase.td"
15+
include "mlir/IR/BuiltinAttributeInterfaces.td"
1516

1617
// All of the attributes will extend this class.
1718
class Ptr_Attr<string name, string attrMnemonic,
@@ -22,35 +23,28 @@ class Ptr_Attr<string name, string attrMnemonic,
2223
}
2324

2425
//===----------------------------------------------------------------------===//
25-
// IAddressSpaceAttr
26+
// GenericSpaceAttr
2627
//===----------------------------------------------------------------------===//
2728

28-
def Ptr_IAddressSpaceAttr :
29-
Ptr_Attr<"IAddressSpace", "int_space", [
29+
def Ptr_GenericSpaceAttr :
30+
Ptr_Attr<"GenericSpace", "generic_space", [
3031
DeclareAttrInterfaceMethods<MemorySpaceAttrInterface>
3132
]> {
32-
let summary = "Int memory space";
33+
let summary = "Generic memory space";
3334
let description = [{
34-
The `int_as` attribute defines a memory space attribute with the following
35-
properties:
35+
The `generic_space` attribute defines a memory space attribute with the
36+
following properties:
3637
- Load and store operations are always valid, regardless of the type.
3738
- Atomic operations are always valid, regardless of the type.
38-
- Cast operations are valid between pointers with `int_space` memory space,
39-
or between non-scalable `vector` of pointers with `int_space` memory space.
40-
41-
The default address spaces is 0.
42-
39+
- Cast operations to `generic_space` are always valid.
40+
4341
Example:
4442

4543
```mlir
46-
// Default address space: 0.
47-
#ptr.int_space
48-
// Address space 3.
49-
#ptr.int_space<3>
44+
#ptr.generic_space
5045
```
5146
}];
52-
let parameters = (ins DefaultValuedParameter<"int64_t", "0">:$value);
53-
let assemblyFormat = "(`<` $value^ `>`)?";
47+
let assemblyFormat = "";
5448
}
5549

5650
//===----------------------------------------------------------------------===//

mlir/include/mlir/Dialect/Ptr/IR/PtrAttrs.h

+3
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,10 @@
1313
#ifndef MLIR_DIALECT_PTR_IR_PTRATTRS_H
1414
#defineMLIR_DIALECT_PTR_IR_PTRATTRS_H
1515

16+
#include"mlir/IR/BuiltinAttributeInterfaces.h"
1617
#include"mlir/IR/OpImplementation.h"
18+
#include"mlir/Interfaces/DataLayoutInterfaces.h"
19+
#include"llvm/Support/TypeSize.h"
1720

1821
#include"mlir/Dialect/Ptr/IR/MemorySpaceInterfaces.h"
1922

mlir/include/mlir/Dialect/Ptr/IR/PtrEnums.td

+1-1
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ def AtomicOrdering : I64EnumAttr<
7070
// Ptr add flags enum attr.
7171
//===----------------------------------------------------------------------===//
7272

73-
def Ptr_PtrAddFlags : I32EnumAttr<"PtrAddFlags", "Pointer add flags", [
73+
def Ptr_PtrAddFlags : I32Enum<"PtrAddFlags", "Pointer add flags", [
7474
I32EnumAttrCase<"none", 0>, I32EnumAttrCase<"nusw", 1>,
7575
I32EnumAttrCase<"nuw", 2>, I32EnumAttrCase<"inbounds", 3>
7676
]> {

mlir/include/mlir/Dialect/Ptr/IR/PtrOps.td

+16-13
Original file line numberDiff line numberDiff line change
@@ -21,40 +21,42 @@ include "mlir/IR/OpAsmInterface.td"
2121
// PtrAddOp
2222
//===----------------------------------------------------------------------===//
2323

24-
def Ptr_PtrAddOp : Pointer_Op<"ptradd", [
25-
Pure, AllTypesMatch<["base", "result"]>,
26-
DeclareOpInterfaceMethods<ViewLikeOpInterface>
24+
def Ptr_PtrAddOp : Pointer_Op<"ptr_add", [
25+
Pure, AllTypesMatch<["base", "result"]>, ViewLikeOpInterface
2726
]> {
2827
let summary = "Pointer add operation";
2928
let description = [{
30-
The `ptradd` operation adds an integer offset to a pointer to produce a new
29+
The `ptr_add` operation adds an integer offset to a pointer to produce a new
3130
pointer. The input and output pointer types are always the same.
3231

3332
Example:
3433

3534
```mlir
36-
%x_off = ptr.ptradd %x, %off : !ptr.ptr<0>, i32
37-
%x_off0 = ptr.ptradd nusw %x, %off : !ptr.ptr<0>, i32
35+
%x_off = ptr.ptr_add %x, %off : !ptr.ptr<0>, i32
36+
%x_off0 = ptr.ptr_add nusw %x, %off : !ptr.ptr<0>, i32
3837
```
3938
}];
4039

4140
let arguments = (ins
4241
Ptr_PtrType:$base,
4342
AnySignlessIntegerOrIndex:$offset,
44-
DefaultValuedAttr<Ptr_PtrAddFlags,
45-
"::mlir::ptr::PtrAddFlags::none">:$flags);
43+
DefaultValuedProp<EnumProp<Ptr_PtrAddFlags>, "PtrAddFlags::none">:$flags);
4644
let results = (outs Ptr_PtrType:$result);
4745
let assemblyFormat = [{
4846
($flags^)? $base `,` $offset attr-dict `:` type($base) `,` type($offset)
4947
}];
5048
let hasFolder = 1;
49+
let extraClassDeclaration = [{
50+
/// `ViewLikeOp::getViewSource` method.
51+
Value getViewSource() { return getBase(); }
52+
}];
5153
}
5254

5355
//===----------------------------------------------------------------------===//
5456
// TypeOffsetOp
5557
//===----------------------------------------------------------------------===//
5658

57-
def Ptr_TypeOffsetOp : Pointer_Op<"type_offset", [ConstantLike, Pure]> {
59+
def Ptr_TypeOffsetOp : Pointer_Op<"type_offset", [Pure]> {
5860
let summary = "Type offset operation";
5961
let description = [{
6062
The `type_offset` operation produces an int or index-typed SSA value
@@ -64,25 +66,26 @@ def Ptr_TypeOffsetOp : Pointer_Op<"type_offset", [ConstantLike, Pure]> {
6466
Example:
6567

6668
```mlir
69+
// Return the offset between two f32 stored in memory
6770
%0 = ptr.type_offset f32 : index
71+
// Return the offset between two memref descriptors stored in memory
6872
%1 = ptr.type_offset memref<12 x f64> : i32
6973
```
7074
}];
7175

72-
let arguments = (ins TypeAttr:$element_type);
76+
let arguments = (ins TypeAttr:$elementType);
7377
let results = (outs AnySignlessIntegerOrIndex:$result);
7478
let builders = [
75-
OpBuilder<(ins "TypeAttr":$element_type)>
79+
OpBuilder<(ins "Type":$elementType)>
7680
];
7781
let assemblyFormat = [{
78-
$element_type attr-dict `:` type($result)
82+
$elementType attr-dict `:` type($result)
7983
}];
8084
let extraClassDeclaration = [{
8185
/// Returns the type offset according to `maybeLayout`. If `maybeLayout` is
8286
/// `nullopt` the nearest layout the op will be used for the computation.
8387
llvm::TypeSize getTypeSize(std::optional<DataLayout> layout = std::nullopt);
8488
}];
85-
let hasFolder = 1;
8689
}
8790

8891
#endif // PTR_OPS

mlir/lib/Dialect/Ptr/IR/PtrAttrs.cpp

+7-7
Original file line numberDiff line numberDiff line change
@@ -20,43 +20,43 @@ using namespace mlir::ptr;
2020
constexprconststaticunsignedkBitsInByte = 8;
2121

2222
//===----------------------------------------------------------------------===//
23-
//IAddressSpaceAttr
23+
//GenericSpaceAttr
2424
//===----------------------------------------------------------------------===//
2525

26-
LogicalResult IAddressSpaceAttr::isValidLoad(
26+
LogicalResult GenericSpaceAttr::isValidLoad(
2727
Type type, ptr::AtomicOrdering ordering, IntegerAttr alignment,
2828
function_ref<InFlightDiagnostic()> emitError) const {
2929
returnsuccess();
3030
}
3131

32-
LogicalResult IAddressSpaceAttr::isValidStore(
32+
LogicalResult GenericSpaceAttr::isValidStore(
3333
Type type, ptr::AtomicOrdering ordering, IntegerAttr alignment,
3434
function_ref<InFlightDiagnostic()> emitError) const {
3535
returnsuccess();
3636
}
3737

38-
LogicalResult IAddressSpaceAttr::isValidAtomicOp(
38+
LogicalResult GenericSpaceAttr::isValidAtomicOp(
3939
ptr::AtomicBinOp op, Type type, ptr::AtomicOrdering ordering,
4040
IntegerAttr alignment, function_ref<InFlightDiagnostic()> emitError) const {
4141
returnsuccess();
4242
}
4343

44-
LogicalResult IAddressSpaceAttr::isValidAtomicXchg(
44+
LogicalResult GenericSpaceAttr::isValidAtomicXchg(
4545
Type type, ptr::AtomicOrdering successOrdering,
4646
ptr::AtomicOrdering failureOrdering, IntegerAttr alignment,
4747
function_ref<InFlightDiagnostic()> emitError) const {
4848
returnsuccess();
4949
}
5050

51-
LogicalResult IAddressSpaceAttr::isValidAddrSpaceCast(
51+
LogicalResult GenericSpaceAttr::isValidAddrSpaceCast(
5252
Type tgt, Type src, function_ref<InFlightDiagnostic()> emitError) const {
5353
// TODO: update this method once the `addrspace_cast` op is added to the
5454
// dialect.
5555
assert(false && "unimplemented, see TODO in the source.");
5656
returnfailure();
5757
}
5858

59-
LogicalResult IAddressSpaceAttr::isValidPtrIntCast(
59+
LogicalResult GenericSpaceAttr::isValidPtrIntCast(
6060
Type intLikeTy, Type ptrLikeTy,
6161
function_ref<InFlightDiagnostic()> emitError) const {
6262
// TODO: update this method once the int-cast ops are added to the dialect.

mlir/lib/Dialect/Ptr/IR/PtrDialect.cpp

-6
Original file line numberDiff line numberDiff line change
@@ -55,16 +55,10 @@ OpFoldResult PtrAddOp::fold(FoldAdaptor adaptor) {
5555
returnnullptr;
5656
}
5757

58-
Value PtrAddOp::getViewSource() { returngetBase(); }
59-
6058
//===----------------------------------------------------------------------===//
6159
// TypeOffsetOp
6260
//===----------------------------------------------------------------------===//
6361

64-
OpFoldResult TypeOffsetOp::fold(FoldAdaptor adaptor) {
65-
returnTypeAttr::get(getElementType());
66-
}
67-
6862
llvm::TypeSize TypeOffsetOp::getTypeSize(std::optional<DataLayout> layout) {
6963
if (layout)
7064
return layout->getTypeSize(getElementType());
+9-27
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,15 @@
11
// RUN: mlir-opt --canonicalize %s | FileCheck %s
22

3-
/// Check `ptradd` and `type_offset` canonicalizer patterns.
3+
/// Check `ptr_add` canonicalizer patterns.
44

55
// CHECK-LABEL: @ops0
6-
func.func@ops0(%ptr:!ptr.ptr<#ptr.int_space<3>>, %c:i1) -> !ptr.ptr<#ptr.int_space<3>> {
7-
// CHECK: (%[[PTR_0:.*]]: !ptr.ptr<#ptr.int_space<3>>,
8-
// CHECK: %[[F32_OFF:.*]] = ptr.type_offset f32 : index
9-
// CHECK: %[[PTR_1:.*]] = ptr.ptradd %[[PTR_0]], %[[F32_OFF]] : <#ptr.int_space<3>>, index
10-
// CHECK: %[[PTR_2:.*]] = ptr.ptradd %[[PTR_1]], %[[F32_OFF]] : <#ptr.int_space<3>>, index
11-
// CHECK: %[[PTR_3:.*]] = scf.if %{{.*}} -> (!ptr.ptr<#ptr.int_space<3>>) {
12-
// CHECK: %[[PTR_4:.*]] = ptr.ptradd %[[PTR_2]], %[[F32_OFF]] : <#ptr.int_space<3>>, index
13-
// CHECK: scf.yield %[[PTR_4]] : !ptr.ptr<#ptr.int_space<3>>
14-
// CHECK: } else {
15-
// CHECK: scf.yield %[[PTR_0]] : !ptr.ptr<#ptr.int_space<3>>
6+
func.func@ops0(%ptr:!ptr.ptr<#ptr.generic_space>) -> !ptr.ptr<#ptr.generic_space> {
7+
// CHECK: (%[[PTR_0:.*]]: !ptr.ptr<#ptr.generic_space>)
8+
// CHECK-NOT: index.constant
9+
// CHECK-NOT: ptr.ptr_add
10+
// CHECK: return %[[PTR_0]] : !ptr.ptr<#ptr.generic_space>
1611
// CHECK: }
17-
// CHECK: return %[[PTR_3]] : !ptr.ptr<#ptr.int_space<3>>
18-
// CHECK: }
19-
%off0 = ptr.type_offsetf32 : index
20-
%res0 = ptr.ptradd%ptr, %off0 : !ptr.ptr<#ptr.int_space<3>>, index
21-
%off1 = ptr.type_offsetf32 : index
22-
%res1 = ptr.ptradd%res0, %off1 : !ptr.ptr<#ptr.int_space<3>>, index
23-
%res3 = scf.if%c -> !ptr.ptr<#ptr.int_space<3>> {
24-
%off2 = ptr.type_offsetf32 : index
25-
%res2 = ptr.ptradd%res1, %off2 : !ptr.ptr<#ptr.int_space<3>>, index
26-
scf.yield%res2 : !ptr.ptr<#ptr.int_space<3>>
27-
} else {
28-
scf.yield%ptr : !ptr.ptr<#ptr.int_space<3>>
29-
}
30-
%off3 = index.constant0
31-
%res4 = ptr.ptradd%res3, %off3 : !ptr.ptr<#ptr.int_space<3>>, index
32-
return%res4 : !ptr.ptr<#ptr.int_space<3>>
12+
%off = index.constant0
13+
%res0 = ptr.ptr_add%ptr, %off : !ptr.ptr<#ptr.generic_space>, index
14+
return%res0 : !ptr.ptr<#ptr.generic_space>
3315
}

mlir/test/Dialect/Ptr/ops.mlir

+12-8
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,19 @@
11
// RUN: mlir-opt %s --verify-roundtrip | FileCheck %s
22

33
/// Check op assembly.
4-
func.func@ops0(%ptr:!ptr.ptr<#ptr.int_space>) -> !ptr.ptr<#ptr.int_space> {
4+
func.func@ops0(%ptr:!ptr.ptr<#ptr.generic_space>) -> !ptr.ptr<#ptr.generic_space> {
55
// CHECK-LABEL: @ops0
66
// CHECK: ptr.type_offset f32 : index
7-
// CHECK-NEXT: ptr.ptradd %{{.*}}, %{{.*}} : <#ptr.int_space>, index
7+
// CHECK-NEXT: ptr.ptr_add %{{.*}}, %{{.*}} : <#ptr.generic_space>, index
8+
// CHECK-NEXT: ptr.ptr_add %{{.*}}, %{{.*}} : <#ptr.generic_space>, index
9+
// CHECK-NEXT: ptr.ptr_add nusw %{{.*}}, %{{.*}} : <#ptr.generic_space>, index
10+
// CHECK-NEXT: ptr.ptr_add nuw %{{.*}}, %{{.*}} : <#ptr.generic_space>, index
11+
// CHECK-NEXT: ptr.ptr_add inbounds %{{.*}}, %{{.*}} : <#ptr.generic_space>, index
812
%off = ptr.type_offsetf32 : index
9-
%res = ptr.ptradd%ptr, %off : !ptr.ptr<#ptr.int_space>, index
10-
%res0 = ptr.ptraddnone%ptr, %off : !ptr.ptr<#ptr.int_space>, index
11-
%res1 = ptr.ptraddnusw%ptr, %off : !ptr.ptr<#ptr.int_space>, index
12-
%res2 = ptr.ptraddnuw%ptr, %off : !ptr.ptr<#ptr.int_space>, index
13-
%res3 = ptr.ptraddinbounds%ptr, %off : !ptr.ptr<#ptr.int_space>, index
14-
return%res : !ptr.ptr<#ptr.int_space>
13+
%res = ptr.ptr_add%ptr, %off : !ptr.ptr<#ptr.generic_space>, index
14+
%res0 = ptr.ptr_addnone%ptr, %off : !ptr.ptr<#ptr.generic_space>, index
15+
%res1 = ptr.ptr_addnusw%ptr, %off : !ptr.ptr<#ptr.generic_space>, index
16+
%res2 = ptr.ptr_addnuw%ptr, %off : !ptr.ptr<#ptr.generic_space>, index
17+
%res3 = ptr.ptr_addinbounds%ptr, %off : !ptr.ptr<#ptr.generic_space>, index
18+
return%res : !ptr.ptr<#ptr.generic_space>
1519
}

0 commit comments

Comments
 (0)
close