[PATCH] D76127: [TableGen] Do not set ReadOnly attribute on intrinsics with side effects

[TOCK Chiu via Phabricator via llvm-commits]

TOCK added a comment.

For example, say we have these intrinsics:

  def int_test_no_mem_v : Intrinsic<[], [llvm_i64_ty], [IntrNoMem, IntrHasSideEffects]>;
  def int_test_read_mem_v : Intrinsic<[], [llvm_i64_ty], [IntrReadMem, IntrHasSideEffects]>;
  def int_test_no_mem : Intrinsic<[llvm_i64_ty], [llvm_i64_ty], [IntrNoMem, IntrHasSideEffects]>;
  def int_test_read_mem : Intrinsic<[llvm_i64_ty], [llvm_i64_ty], [IntrReadMem, IntrHasSideEffects]>;

Following code:

  define i64 @foo(i64 %a, i64 %b) {
  entry:
    call void @llvm.test.no.mem.v(i64 %a)
    call void @llvm.test.no.mem.v(i64 %a)
    call void @llvm.test.read.mem.v(i64 %b)
    call void @llvm.test.read.mem.v(i64 %b)
    %r1 = call i64 @llvm.test.no.mem(i64 %a)
    %r3 = call i64 @llvm.test.no.mem(i64 %a)
    %r2 = call i64 @llvm.test.read.mem(i64 %b)
    %r4 = call i64 @llvm.test.read.mem(i64 %b)
    %r12 = add nsw i64 %r1, %r2
    %r34 = add nsw i64 %r3, %r4
    %r14 = add nsw i64 %r12, %r34
    ret i64 %r14
  }
  declare void @llvm.test.no.mem.v(i64)
  declare void @llvm.test.read.mem.v(i64)
  declare i64 @llvm.test.no.mem(i64)
  declare i64 @llvm.test.read.mem(i64)

would be optimized into:

  define i64 @foo(i64 %a, i64 %b) local_unnamed_addr #0 {
  entry:
    tail call void @llvm.test.no.mem.v(i64 %a)
    tail call void @llvm.test.no.mem.v(i64 %a)
    %r1 = tail call i64 @llvm.test.no.mem(i64 %a)
    %r3 = tail call i64 @llvm.test.no.mem(i64 %a)
    %r2 = tail call i64 @llvm.test.read.mem(i64 %b)
    %factor = shl i64 %r2, 1
    %r12 = add i64 %r3, %r1
    %r14 = add i64 %r12, %factor
    ret i64 %r14
  }
  ...

Calls to `@llvm.test.read.mem.v` are removed completely (this is what D64414 <https://reviews.llvm.org/D64414> tried to address), also one call to `@llvm.test.read.mem` is removed:

  EarlyCSE DCE:   call void @llvm.test.read.mem.v(i64 %b)                                                                                           
  EarlyCSE DCE:   call void @llvm.test.read.mem.v(i64 %b)                                                                                           
  EarlyCSE CSE CALL:   %r4 = call i64 @llvm.test.read.mem(i64 %b)  to:   %r2 = call i64 @llvm.test.read.mem(i64 %b)                                 
          discovered a new reachable node %entry

This is due to the fact that TableGen doesn't respect `IntrHasSideEffects`, and make it read only:

  ...
      case 52: {
        const Attribute::AttrKind Atts[] = {Attribute::NoUnwind,Attribute::ReadOnly};
        AS[0] = AttributeList::get(C, AttributeList::FunctionIndex, Atts);
        NumAttrs = 1;
        break;
        }
  ...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76127/new/

https://reviews.llvm.org/D76127