This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] [TableGen] Modify RISCVCompressInstEmitter.cpp to use getAllDerivedDefinitions()
ClosedPublic

Authored by Paul-C-Anagnostopoulos on Oct 13 2020, 10:08 AM.

Details

Summary

This patch modifies the TableGen backend RISCVCompressInstEmmiter.cpp to use the getAllDerivedDefinitions() functions to get its working records. Before this patch, it filtered the records by hand.

This patch was separated out from a previous TableGen-related patch.

Diff Detail

Event Timeline

Paul-C-Anagnostopoulos requested review of this revision.Oct 13 2020, 10:08 AM
lattner accepted this revision.Oct 13 2020, 11:16 AM

Looks obvious, feel free to directly commit things that you are confident are clear improvements.

This revision is now accepted and ready to land.Oct 13 2020, 11:16 AM
asb accepted this revision.Oct 15 2020, 3:18 AM

LGTM, thanks Paul.

Patch has been pushed.

As I said in D88832:

Please use arc patch next time to get the Differential Revision: header so the commit and the revision are linked in both directions, which will also automatically close the revision.

(so please close this revision now and follow the right process in future)

I do not want the revision to close when I push the patch. I'm using Phabricator to keep track of the status of each patch, so I want to close the revision once I know that the patch built correctly.

Can I cross-link the commit and revision without closing the revision?

I do not want the revision to close when I push the patch. I'm using Phabricator to keep track of the status of each patch, so I want to close the revision once I know that the patch built correctly.

Can I cross-link the commit and revision without closing the revision?

Please do not do that, that is not how Phabricator is meant to be used. You can update the Phabricator metadata, but you cannot update the commit once pushed, and it is really important that people be able to look at a commit and get the link to the Phabricator review in order to be able to see the discussion for why the patch ended up the way it did.

As for building correctly:

You should be building the patch and running the test suite locally. It is possible that one of the buildbots will spot an error in a different configuration than yours, in which case you can do one of

  • direct commit a fix if it's sufficiently obvious; or
  • post a fix for quick review as a new patch if the breakage is limited; or
  • revert the commit and then reopen the revision here with an updated patch.

Yes, I have been running the TableGen test suite and building all the targets to test my patches. I've added tests as appropriate.

I will use arc patch from now on.

Oh, and thanks for the list of approaches to dealing with a build problem.