Fix mkldnn related irpasses being added triple times

[LeoZhao-Intel]

test=develop

resolve #16174

[CLAassistant]

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[LeoZhao-Intel]

start review @kbinias @Superjomn

[luotao1]

Since #16174 (comment), how about fix "The rootcause is CpuPassStrategy uses copy constructor to re-initialize PassStrategy by update() function, the passes in previous instance is still kept"?

[LeoZhao-Intel]

Since #16174 (comment), how about fix "The rootcause is CpuPassStrategy uses copy constructor to re-initialize PassStrategy by update() function, the passes in previous instance is still kept"?

In AnalysisConfig::Update(), pass_builder_ will be reset like this:

 } else {
    if (use_gpu()) {
      pass_builder_.reset(new GpuPassStrategy(
          *static_cast<GpuPassStrategy *>(pass_builder_.get())));

    } else {
      **pass_builder_.reset(new CpuPassStrategy(
          *static_cast<CpuPassStrategy *>(pass_builder_.get())));
    }**
 }

it uses copy constructor to reconstruct passstrategy, and passes_ is kept like

class CpuPassStrategy : public PassStrategy {
 public:
  CpuPassStrategy();

  explicit CpuPassStrategy(const CpuPassStrategy &other)
      : PassStrategy(**other.AllPasses()**) {}

so when call EnableMKLDNN(), the mkldnn irpassed are inserted into old passes vector instead of null vector, my patch will check if head has already include mkldnn irpass, if not, it will do insertion.

Not sure for the original purpose of CpuPassStrategy copy constructor, if not use copy constructor, the change may be big and not sure if impact other cases

[LeoZhao-Intel]

void AnalysisConfig::EnableMKLDNN() {
#ifdef PADDLE_WITH_MKLDNN
  pass_builder()->EnableMKLDNN();
  use_mkldnn_ = true;
#else
  LOG(ERROR) << "Please compile with MKLDNN first to use MKLDNN";
  use_mkldnn_ = false;
#endif

  Update();
}

pass_builder()->EnableMKLDNN() is called again in Update() after pass_builder reset

[luotao1]

Not sure for the original purpose of CpuPassStrategy copy constructor, if not use copy constructor, the change may be big and not sure if impact other cases

@Superjomn Could you help see #16175 (comment)?

[luotao1]

Thanks for your contribution, we will consider to solve it in general.

[sfraczek]

I was thinking that the copy constructor could copy use_mkldnn_ flag and it would also be enough. I ran into this error to and I was wondering if it is on purpose that the copy constructor does a copy of the passes only.

[LeoZhao-Intel]

I was thinking that the copy constructor could copy use_mkldnn_ flag and it would also be enough. I ran into this error to and I was wondering if it is on purpose that the copy constructor does a copy of the passes only.

Yes, but unfortunately this copy constructor just copies passes_

[luotao1]

@LeoZhao-Intel @sfraczek Could you help see #16225? I fix it in another way.

[LeoZhao-Intel]

@LeoZhao-Intel @sfraczek Could you help see #16225? I fix it in another way.

seems not working, pass_builder_ will be reset to new instance with just passes_ kept, "use_mkldnn_" value will be lost in new PassStrategy, PassStrategy copy constructor just copies passes_.

If do change like #16225, PassStrategy copy constructor need do more.

[wojtuss]

@LeoZhao-Intel @sfraczek Could you help see #16225? I fix it in another way.

seems not working, pass_builder_ will be reset to new instance with just passes_ kept, "use_mkldnn_" value will be lost in new PassStrategy, PassStrategy copy constructor just copies passes_.

As @sfraczek sugested (#16175 (comment)) you could make the copy constructor copy also the use_mkldnn_ value.

[luotao1]

close due to #16606