This is an archive of the discontinued LLVM Phabricator instance.

[Triple] Add Facebook vendor
ClosedPublic

Authored by smeenai on Sep 30 2016, 3:10 PM.

Details

Summary

Add a compiler vendor for Facebook, to enable future vendor-specific
behavior.

Diff Detail

Repository
rL LLVM

Event Timeline

smeenai updated this revision to Diff 73153.Sep 30 2016, 3:10 PM
smeenai retitled this revision from to [Triple] Add Facebook vendor.
smeenai updated this object.
smeenai added a reviewer: compnerd.
smeenai added a subscriber: llvm-commits.
davide added a subscriber: davide.Sep 30 2016, 4:27 PM

The patch has no tests associated. Also, can you please state what's your long(er) term plan?
As it stands, this is pretty much dead code, unless there's something else coming soon in the pipeline.

compnerd edited edge metadata.Oct 1 2016, 8:47 AM

The patch implicitly has tests: the unit tests generically walks over the enumeration, so it is covered by existing tests.

This would allow Facebook to play around with changes that they need for local development, which may not necessarily be generic enough or generally correct for merging into the mainline.

I don't think that there is any real maintenance cost associated with adding the additional vendor triple, especially if it doesnt require altering the normal code paths.

compnerd accepted this revision.Nov 9 2016, 4:34 PM
compnerd edited edge metadata.

Spoke with @davide offline. He's fine with with this, was just curious what the intention here was.

This revision is now accepted and ready to land.Nov 9 2016, 4:34 PM

To address @dberlin's comments on the mailing list (I've lost the original mail so I can't reply directly):

So why not keep this patch local as well?
In particular, if this is for local development, keeping this patch local
also helps enforce this since any accidental use of the triple in public
code will break quite obviously.

The intent is to be able to upstream anything generic enough to be generally useful, even where the behavior is vendor-specific.

This revision was automatically updated to reflect the committed changes.