clangir icon indicating copy to clipboard operation
clangir copied to clipboard

Add an address point attribute

Open bcardosolopes opened this issue 1 year ago • 4 comments

We should teach GlobalViewAttr and VTableAddrPointOp to use them instead.

Originally posted by @bcardosolopes in https://github.com/llvm/clangir/pull/252#discussion_r1323843899

bcardosolopes avatar Sep 13 '23 22:09 bcardosolopes

@bcardosolopes can you please mention a good start point, thanks

elhewaty avatar Jan 15 '24 16:01 elhewaty

@bcardosolopes ping

elhewaty avatar Mar 04 '24 23:03 elhewaty

Sorry, ended up missing this. A good starting PR would be:

  1. Create a new attribute, let's say, #cir.address_point or something similar.
  2. Change VTableAddrPointOp definition in CIROps.td to use that as input instead of I32Attr:$vtable_index, I32Attr:$address_point_index.

GlobalViewAttr could be handled in a follow up PR.

bcardosolopes avatar Mar 06 '24 21:03 bcardosolopes

hi @bcardosolopes I'd like to share my understanding of the issue.

Currently, vtable.address_point has three usages: Depending on the first parameter in the op, it can either be (1) a global variable representing the vtable,

%6 = cir.vtable.address_point(@_ZTV5Child, vtable_index = 0, address_point_index = 2) : !cir.ptr<!cir.ptr<!cir.func<!u32i ()>>>

or (2) a value representing the vptr. IIUC, the vtable_index should always be zero.

 %10 = cir.vtable.address_point( %9 : !cir.ptr<!cir.ptr<!cir.func<!void (!cir.ptr<!ty_22Mother22>)>>>, vtable_index = 0, address_point_index = 2) : !cir.ptr<!cir.ptr<!cir.func<!void (!cir.ptr<!ty_22Mother22>)>>>

(3)In the VTT (Virtual Table Table), the address of a vtable can be stored. However, it currently still uses the generic #cir.global_view to access vtable elements.

cir.global linkonce_odr @_ZTT1D = #cir.const_array<[
  #cir.global_view<@_ZTV1D, [0 : i32, 0 : i32, 3 : i32]> : !cir.ptr<!u8i>, 
  #cir.global_view<@_ZTC1D0_1B, [0 : i32, 0 : i32, 3 : i32]> : !cir.ptr<!u8i>, 
  #cir.global_view<@_ZTC1D0_1B, [0 : i32, 1 : i32, 3 : i32]> : !cir.ptr<!u8i>,
  ...
]> : !cir.array<!cir.ptr<!u8i> x 7> {alignment = 8 : i64} loc(#loc36)

In my view there are two proposals to handle it:

  1. As you mentioned, create a new attribute #cir.address_point instead of I32Attr:$vtable_index and I32Attr:$address_point_index , following:

(1)

%6 = cir.vtable.address_point(@_ZTV5Child, #cir.address_point(vtable_index = 0, address_point_index = 2)) : !cir.ptr<!cir.ptr<!cir.func<!u32i ()>>>

(2)

 %10 = cir.vtable.address_point( %9 : !cir.ptr<!cir.ptr<!cir.func<!void (!cir.ptr<!ty_22Mother22>)>>>, #cir.address_point(vtable_index = 0, address_point_index = 2)) : !cir.ptr<!cir.ptr<!cir.func<!void (!cir.ptr<!ty_22Mother22>)>>>

(3)

cir.global linkonce_odr @_ZTT1D = #cir.const_array<[
  #cir.global_view<@_ZTV1D, #cir.address_point(vtable_index = 0, address_point_index = 2)> : !cir.ptr<!u8i>, 
  #cir.global_view<@_ZTC1D0_1B, #cir.address_point(vtable_index = 0, address_point_index = 3)> : !cir.ptr<!u8i>, 
  #cir.global_view<@_ZTC1D0_1B, #cir.address_point(vtable_index = 1, address_point_index = 3)> : !cir.ptr<!u8i>,
  ...
]> : !cir.array<!cir.ptr<!u8i> x 7> {alignment = 8 : i64} loc(#loc36)

I feel this doesn't make much of a difference in using VTT or Vtable compared to before.

  1. I'd like to create a new attribute similar to vtable.address_point op, #cir.vtable.address_point(Or is there a better name?), which is primarily designed to only replace #cir.global_view in (3).
cir.global linkonce_odr @_ZTT1D = #cir.const_array<[
  #cir.vtable.address_point(@_ZTV1D, vtable_index = 0, address_point_index = 2),
  #cir.vtable.address_point(@_ZTC1D0_1B, vtable_index = 0, address_point_index = 3),
  #cir.vtable.address_point(@_ZTC1D0_1B, vtable_index = 1, address_point_index = 3)
  ...
]> : !cir.array<!cir.ptr<!u8i> x 7> {alignment = 8 : i64} loc(#loc36)

Going further, The vtable.address_point op also supports this new attribute in usage (1). While retaining the use of offsets for vptr in usage (2).

%6 = cir.vtable.address_point(#cir.vtable.address_point(@_ZTV1D, vtable_index = 0, address_point_index = 2)) : !cir.ptr<!cir.ptr<!cir.func<!u32i ()>>>
 %10 = cir.vtable.address_point( %9 : !cir.ptr<!cir.ptr<!cir.func<!void (!cir.ptr<!ty_22Mother22>)>>>, vtable_index = 0, address_point_index = 0) : !cir.ptr<!cir.ptr<!cir.func<!void (!cir.ptr<!ty_22Mother22>)>>>

Or Split vtable.address_point op into two new op?

 %10 = cir.vtable.address_point(#cir.vtable.component(@_ZTV1D, vtable_index = 0, address_point_index = 2)) : !cir.ptr<!cir.ptr<!cir.func<!void (!cir.ptr<!ty_22Mother22>)>>>
 %10 = cir.vptr.address_point( %9 : !cir.ptr<!cir.ptr<!cir.func<!void (!cir.ptr<!ty_22Mother22>)>>>, address_point_index = 1) : !cir.ptr<!cir.ptr<!cir.func<!void (!cir.ptr<!ty_22Mother22>)>>>

The vtable_index can be removed in this op

What do you think?

Laity000 avatar May 30 '24 15:05 Laity000