This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Split RISCVISelDAGToDAG.cpp to RISCVISelDAGToDAG.h and RISCVISelDAGToDAG.cpp
ClosedPublic

Authored by shiva0217 on Mar 31 2020, 12:08 AM.

Details

Summary

For the downstream RISCV maintenance, it would be easier to inherent RISCVISelDAGToDAG by including header and only override the method that needs to be customized for the provider non-standard ISA extension without touching RISCVISelDAGToDAG.cpp which may cause conflict when upgrading the downstream LLVM version.

Diff Detail

Event Timeline

shiva0217 created this revision.Mar 31 2020, 12:08 AM

Hi @shiva0217 can you clarify as to why this is necessary?

Hi @shiva0217 can you clarify as to why this is necessary?

Hi @lenary,
For the downstream RISCV maintenance, it would be easier to inherent the RISCVISelDAGToDAG and only override the method need to customize for the custom provider ISA extension without touching RISCVISelDAGToDAG.cpp which may cause conflict when upgrading the downstream LLVM version.
I think it could be helpful for other RISCV downstream maintenance. Could it be the sufficient reason to split out the header?

For the downstream RISCV maintenance, it would be easier to inherent the RISCVISelDAGToDAG and only override the method need to customize for the custom provider ISA extension without touching RISCVISelDAGToDAG.cpp which may cause conflict when upgrading the downstream LLVM version.
I think it could be helpful for other RISCV downstream maintenance. Could it be the sufficient reason to split out the header?

That sounds reasonable.

lenary accepted this revision.Mar 31 2020, 10:48 AM

@shiva0217 yeah, that sounds good. Please can you ensure you include some of that explanation in the commit message when you land this?

This revision is now accepted and ready to land.Mar 31 2020, 10:48 AM
shiva0217 edited the summary of this revision. (Show Details)Mar 31 2020, 7:56 PM

@shiva0217 yeah, that sounds good. Please can you ensure you include some of that explanation in the commit message when you land this?

Sure, thanks.

For the downstream RISCV maintenance, it would be easier to inherent the RISCVISelDAGToDAG and only override the method need to customize for the custom provider ISA extension without touching RISCVISelDAGToDAG.cpp which may cause conflict when upgrading the downstream LLVM version.
I think it could be helpful for other RISCV downstream maintenance. Could it be the sufficient reason to split out the header?

That sounds reasonable.

Thanks for the support.

This revision was automatically updated to reflect the committed changes.