This is an archive of the discontinued LLVM Phabricator instance.

handle armeb/thumb/thumbeb consistently in gnutools::Assemble::ConstructJob
ClosedPublic

Authored by scott-0 on Mar 10 2015, 4:35 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

scott-0 updated this revision to Diff 21565.Mar 10 2015, 4:35 AM
scott-0 retitled this revision from to handle armeb/thumb/thumbeb consistently in gnutools::Assemble::ConstructJob.
scott-0 updated this object.
scott-0 edited the test plan for this revision. (Show Details)
scott-0 added a subscriber: Unknown Object (MLST).
joerg added a subscriber: joerg.Mar 10 2015, 6:09 AM

Can you convert it to switch over the arch instead? That's much more concise and readable for cases like this.

Thanks. Now that I've found it, it looks like switching on Triple::getSubArch() would be better; I'll try that and update this patch.

scott-0 updated this revision to Diff 21587.Mar 10 2015, 8:55 AM

Is this what you had in mind?

Or did you mean to change all of gnutools::Assemble::ConstructJob() to be a switch(getToolChain().getArch())? I'm happy to do that, too, as a follow up.

joerg added a comment.Mar 10 2015, 9:05 AM

That and the rest of the function, yes.

Ok, thanks. I'd rather do the whole function as a separate, no-functional-change commit. Are you happy with this as-is?

I think it is the correct approach, but Tim might be a better person to judge the subarch part.

scott-0 updated this revision to Diff 22357.Mar 20 2015, 10:19 AM

rebased to be after cleanup in http://reviews.llvm.org/D8485

rengolin accepted this revision.Mar 21 2015, 8:43 AM
rengolin added a reviewer: rengolin.

This looks a lot better, thanks Scott! LGTM, too.

This revision is now accepted and ready to land.Mar 21 2015, 8:43 AM
This revision was automatically updated to reflect the committed changes.