This is an archive of the discontinued LLVM Phabricator instance.

[gold] Simplify with StringRef::consume_front. NFC
ClosedPublic

Authored by MaskRay on Apr 24 2020, 10:47 AM.

Diff Detail

Event Timeline

MaskRay created this revision.Apr 24 2020, 10:47 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 24 2020, 10:47 AM

Thanks for the nice cleanup. I spotted an issue though, see below.

llvm/tools/gold/gold-plugin.cpp
268–269

I think this one needs a fix, presumably just Num(opt_) now?

Looks like there isn't a good way to test this unfortunately, unless the garbage read causes the get_threadpool_strategy call below to fail. Can you find or add a way to test this parameter?

MaskRay marked an inline comment as done.Apr 24 2020, 12:24 PM
MaskRay added inline comments.
llvm/tools/gold/gold-plugin.cpp
268–269

This is correct, because opt_ is the original const char * (unmodified). As you said, using opt is better. I'll fix that. Actually, 4 tests check that we can't pass garbage to get_threadpool_strategy:

/usr/bin/ld.gold: fatal error: Invalid parallelism level: jobs=1

--

********************
********************
Failing Tests (4):
  LLVM :: tools/gold/X86/thinlto.ll
  LLVM :: tools/gold/X86/thinlto_afdo.ll
  LLVM :: tools/gold/X86/thinlto_archive.ll
  LLVM :: tools/gold/X86/thinlto_cspgo.ll
MaskRay updated this revision to Diff 259963.Apr 24 2020, 12:27 PM
MaskRay marked an inline comment as done.

Improve jobs= handling

tejohnson accepted this revision.Apr 24 2020, 12:29 PM

lgtm

llvm/tools/gold/gold-plugin.cpp
268–269

Ah right, missed the fact that it used the original char pointer. Glad the tests catch any issues here.

This revision is now accepted and ready to land.Apr 24 2020, 12:29 PM
This revision was automatically updated to reflect the committed changes.
MaskRay marked an inline comment as not done.