Redundant calls in in generated DRR code

During the rewrite phase of the DRR, it appears that custom operations that create an Value, for example, are called twice. When doing constant folding, this means that the same constant will be created twice. This seems superfluous. Are there general objections in creating the value once, testing if non-null, and then using it? Code pattern is shown below (look at ConstPropTranspose).

    // Rewrite
    auto odsLoc = rewriter.getFusedLoc({tblgen_ops[0]->getLoc(), tblgen_ops[1]->getLoc()}); (void)odsLoc;
    SmallVector<Value, 4> tblgen_repl_values;
    ONNXConstantOp tblgen_ONNXConstantOp_0;
      SmallVector<Value, 4> tblgen_values; (void)tblgen_values;
      SmallVector<NamedAttribute, 4> tblgen_attrs; (void)tblgen_attrs;
      if (Attribute()) {  tblgen_attrs.emplace_back(rewriter.getIdentifier("sparse_value"), Attribute()); }
      if (ConstPropTranspose(rewriter, (*resOp.getODSResults(0).begin()), v, p)) {  tblgen_attrs.emplace_back(rewriter.getIdentifier("value"), ConstPropTranspose(rewriter, (*resOp.getODSResults(0).begin()), v, p)); }
      SmallVector<Type, 4> tblgen_types; (void)tblgen_types;
      for (auto v : castedOp0.getODSResults(0)) {tblgen_types.push_back(v.getType()); }
      tblgen_ONNXConstantOp_0 = rewriter.create<ONNXConstantOp>(odsLoc, tblgen_types, tblgen_values, tblgen_attrs);

The pattern that generated this is below.

def TransposeofConst :  Pat<
    // From TransposeOp(c, p)
    (ONNXTransposeOp:$resOp (ONNXConstantOp $s, $v), $p),
    // To c' where c' is transposed attribute
    (ONNXConstantOp (GetNullAttr), (CreateTransposeOfConst $resOp, $v, $p)),

Happy to look into it if you agree the code could be called only once.

Is this the call due to CreateTransposeOfConst ?

In the above it definitely seem like we could do that (especially given our Attribute behavior). I’d need to dig more once back to see why, but this seems wrong and it would be great if you could take a look (and I’d be happy to help too once I’m back).

Yes, the TableGen def CreateTransposeOfConst is in turn calling the ConstPropTranspose C++ call.

def CreateTransposeOfConst :
   NativeCodeCall<"ConstPropTranspose($_builder, $0, $1, $2)">;

Happy to look into removing the redundancy if there is no underlying reason to have it (it appears that there are none)

create PR #223 with the changes to remove redundancy.

Thanks we’ll take a look soon. As the bot mentioned GitHub side, these should be sent via phabricator for review. Let me know if you have any questions/need help there.

My bad, here it is:

Need help to drop the code, thanks

Sure, I submitted this Monday morning (I ran my submit script but forgot to hit enter and so saw terminal waiting for input on Monday morning …)