Page MenuHomePhabricator

[ifs] Prepare llvm-ifs for elfabi/ifs merging.
Needs ReviewPublic

Authored by haowei on Fri, Apr 2, 12:02 PM.

Details

Summary

This diff changes llvm-ifs to use unified IFS file format and perform other renaming changes in preparation for the merging between elfabi/ifs.

Diff Detail

Unit TestsFailed

TimeTest
1,070 msx64 debian > libomp.lock::omp_init_lock.c
Script: -- : 'RUN: at line 1'; /mnt/disks/ssd0/agent/llvm-project/build/./bin/clang -fopenmp -pthread -fno-experimental-isel -I /mnt/disks/ssd0/agent/llvm-project/openmp/runtime/test -I /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/runtime/src -L /mnt/disks/ssd0/agent/llvm-project/build/lib -I /mnt/disks/ssd0/agent/llvm-project/openmp/runtime/test/ompt /mnt/disks/ssd0/agent/llvm-project/openmp/runtime/test/lock/omp_init_lock.c -o /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/runtime/test/lock/Output/omp_init_lock.c.tmp -lm -latomic && /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/runtime/test/lock/Output/omp_init_lock.c.tmp

Event Timeline

haowei created this revision.Fri, Apr 2, 12:02 PM
haowei requested review of this revision.Fri, Apr 2, 12:02 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptFri, Apr 2, 12:02 PM
haowei updated this revision to Diff 335038.Fri, Apr 2, 3:44 PM

Addressed clang tests.

This change is pretty large so I'm wondering if we could possibly split it into two changes, one that does the purely mechanical renaming of ELF and TBE to IFS, and second one that merges the two tools?

clang/lib/Driver/ToolChains/InterfaceStubs.cpp
26 ↗(On Diff #335038)

Do we know whether the binary output format for the current target is ELF? Shouldn't we check it first?

llvm/include/llvm/InterfaceStub/IFSStub.h
28–52

Since IFS is supposed to be binary format agnostic, it's a bit confusing to use ELF constants here. Could we instead define these values ourselves and convert them to the corresponding ELF values when generating the stub?

42
50
llvm/tools/llvm-elfabi/llvm-elfabi.cpp
82–83
llvm/tools/llvm-ifs/llvm-ifs.cpp
202
204
399
565

The case difference on the left and right side (SoName vs SOName) is confusing, I'd unify them.

570–572

No need for curly braces.

haowei updated this revision to Diff 335709.Tue, Apr 6, 8:34 PM
haowei marked 5 inline comments as done.
haowei marked 3 inline comments as done.Tue, Apr 6, 8:38 PM

I will try to split this change.

clang/lib/Driver/ToolChains/InterfaceStubs.cpp
26 ↗(On Diff #335038)

I tried following way:

const bool WriteBin = !Args.getLastArg(options::OPT_emit_merged_ifs);
  if (WriteBin) {
    llvm::Triple::OSType OS = C.getDefaultToolChain().getEffectiveTriple().getOS();
    switch(OS) {
    case llvm::Triple::OSType::DragonFly:
    case llvm::Triple::OSType::Fuchsia:
    case llvm::Triple::OSType::KFreeBSD:
    case llvm::Triple::OSType::Linux:
    case llvm::Triple::OSType::Lv2:
    case llvm::Triple::OSType::NetBSD:
    case llvm::Triple::OSType::OpenBSD:
    case llvm::Triple::OSType::Solaris:
    case llvm::Triple::OSType::Haiku:
    case llvm::Triple::OSType::Minix:
    case llvm::Triple::OSType::NaCl:
    case llvm::Triple::OSType::PS4:
    case llvm::Triple::OSType::ELFIAMCU:
    case llvm::Triple::OSType::Contiki:
    case llvm::Triple::OSType::Hurd:
      CmdArgs.push_back("--output-format=ELF");
      break;
    // default:
    //   // Not adding "--output-format" will cause llvm-ifs to crash.
    }
  } else {
    CmdArgs.push_back("--output-format=IFS");
  }

but it does not work. It looks like the Toolchain object is not properly constructed in this stage, which makes it difficult to determine the target of current clang invocation. Do you have any suggestions?

llvm/include/llvm/InterfaceStub/IFSStub.h
28–52

I removed ELF constants and added helper functions.

haowei updated this revision to Diff 336213.Thu, Apr 8, 1:50 PM
haowei marked an inline comment as done.
haowei retitled this revision from [ifs][elfabi] Merge llvm-elfabi tool into llvm-ifs to [ifs] Prepare llvm-ifs for elfabi/ifs merging..
haowei edited the summary of this revision. (Show Details)

Split this change into 2.