Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

llvm: cleanup options parsing #42413

Merged
merged 1 commit into from
Sep 29, 2021
Merged

llvm: cleanup options parsing #42413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 14 additions & 34 deletions src/codegen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8209,42 +8209,20 @@ extern "C" void jl_init_llvm(void)
#endif

// Parse command line flags after initialization
const char *const argv_tailmerge[] = {"", "-enable-tail-merge=0"}; // NOO TOUCHIE; NO TOUCH! See #922
cl::ParseCommandLineOptions(sizeof(argv_tailmerge)/sizeof(argv_tailmerge[0]), argv_tailmerge, "disable-tail-merge\n");
#if defined(_OS_WINDOWS_) && defined(_CPU_X86_64_)
const char *const argv_copyprop[] = {"", "-disable-copyprop"}; // llvm bug 21743
cl::ParseCommandLineOptions(sizeof(argv_copyprop)/sizeof(argv_copyprop[0]), argv_copyprop, "disable-copyprop\n");
Comment on lines -8215 to -8216
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://bugs.llvm.org/show_bug.cgi?id=21743 fixed by

commit 796d906e06f8f5a3c8d4f5f0faa5c47697153490
Author: Quentin Colombet <qcolombet@apple.com>
Date:   Thu Apr 23 21:17:39 2015 +0000

    [MachineCopyPropagation] Handle undef flags conservatively so that we do not
    remove copies that are useful after breaking some hardware dependencies.
    In other words, handle this kind of situations conservatively by assuming reg2
    is redefined by the undef flag.
    reg1 = copy reg2
    = inst reg2<undef>
    reg2 = copy reg1
    Copy propagation used to remove the last copy.
    This is incorrect because the undef flag on reg2 in inst, allows next
    passes to put whatever trashed value in reg2 that may help.
    In practice we end up with this code:
    reg1 = copy reg2
    reg2 = 0
    = inst reg2<undef>
    reg2 = copy reg1
    
    This fixes PR21743.
    
    llvm-svn: 235647

#endif
#if defined(_CPU_X86_) || defined(_CPU_X86_64_)
const char *const argv_avoidsfb[] = {"", "-x86-disable-avoid-SFB"}; // llvm bug 41629, see https://gist.github.com/vtjnash/192cab72a6cfc00256ff118238163b55
cl::ParseCommandLineOptions(sizeof(argv_avoidsfb)/sizeof(argv_avoidsfb[0]), argv_avoidsfb, "disable-avoidsfb\n");
Comment on lines -8219 to -8220
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://bugs.llvm.org/show_bug.cgi?id=41629 fixed by

$ git show 8172ed91f8ff753fb
commit 8172ed91f8ff753fb3042f39b0d43aed89fdd3e6
Author: Craig Topper <craig.topper@intel.com>
Date:   Wed Jun 24 00:10:36 2020 -0700

    [X86] Speculatively fix to X86AvoidStoreForwardingBlocks not deference a machine mem operand if there isn't one present.
    
    Eric Christopher informed me that FastISel memcpy handling creates
    load/store instructions without mem operands. We should fix that,
    but I doubt that's the only case of missed mem operands so seems
    better to be defensive here.
    
    I don't have a test case yet, but I'll try to add one if i get a
    test from Eric.

#endif
#if JL_LLVM_VERSION >= 120000
// https://reviews.llvm.org/rGc068e9c8c123e7f8c8f3feb57245a012ccd09ccf
Optional<std::string> envValue = sys::Process::GetEnv("JULIA_LLVM_ARGS");
if (envValue) {
SmallVector<const char *, 20> newArgv;
BumpPtrAllocator A;
StringSaver Saver(A);
newArgv.push_back(Saver.save("Julia").data());

// Parse the value of the environment variable into a "command line"
// and hand it off to ParseCommandLineOptions().
cl::TokenizeGNUCommandLine(*envValue, Saver, newArgv);
int newArgc = static_cast<int>(newArgv.size());
cl::ParseCommandLineOptions(newArgc, &newArgv[0]);
}
#else
cl::ParseEnvironmentOptions("Julia", "JULIA_LLVM_ARGS");
#endif

StringMap<cl::Option*> &llvmopts = cl::getRegisteredOptions();
const char *const argv[1] = {"julia"};
cl::ParseCommandLineOptions(1, argv, "", nullptr, "JULIA_LLVM_ARGS");
vchuravy marked this conversation as resolved.
Show resolved Hide resolved

// Set preferred non-default options
cl::Option *clopt;
clopt = llvmopts.lookup("enable-tail-merge"); // NOO TOUCHIE; NO TOUCH! See #922
if (clopt->getNumOccurrences() == 0)
cl::ProvidePositionalOption(clopt, "0", 1);
// if the patch adding this option has been applied, lower its limit to provide
// better DAGCombiner performance.
auto &clOptions = cl::getRegisteredOptions();
if (clOptions.find("combiner-store-merge-dependence-limit") != clOptions.end()) {
const char *const argv_smdl[] = {"", "-combiner-store-merge-dependence-limit=4"};
cl::ParseCommandLineOptions(sizeof(argv_smdl)/sizeof(argv_smdl[0]), argv_smdl);
}
clopt = llvmopts.lookup("combiner-store-merge-dependence-limit");
if (clopt && clopt->getNumOccurrences() == 0)
cl::ProvidePositionalOption(clopt, "4", 1);

TargetOptions options = TargetOptions();
//options.PrintMachineCode = true; //Print machine code produced during JIT compiling
Expand Down Expand Up @@ -8346,6 +8324,8 @@ extern "C" void jl_init_llvm(void)
if (jl_using_perf_jitevents)
jl_ExecutionEngine->RegisterJITEventListener(JITEventListener::createPerfJITEventListener());
#endif

cl::PrintOptionValues();
}

extern "C" void jl_init_codegen(void)
Expand Down
20 changes: 20 additions & 0 deletions test/cmdlineargs.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,26 @@ let exename = `$(Base.julia_cmd()) --startup-file=no --color=no`
@test v[2] == "1"
@test isempty(v[3])
end

let v = readchomperrors(setenv(`$exename -e 0`, "JULIA_LLVM_ARGS" => "-print-options", "HOME" => homedir()))
@test v[1]
@test contains(v[2], r"print-options + = 1")
@test contains(v[2], r"combiner-store-merge-dependence-limit + = 4")
@test contains(v[2], r"enable-tail-merge + = 2")
@test isempty(v[3])
end
let v = readchomperrors(setenv(`$exename -e 0`, "JULIA_LLVM_ARGS" => "-print-options -enable-tail-merge=1 -combiner-store-merge-dependence-limit=6", "HOME" => homedir()))
@test v[1]
@test contains(v[2], r"print-options + = 1")
@test contains(v[2], r"combiner-store-merge-dependence-limit + = 6")
@test contains(v[2], r"enable-tail-merge + = 1")
@test isempty(v[3])
end
let v = readchomperrors(setenv(`$exename -e 0`, "JULIA_LLVM_ARGS" => "-print-options -enable-tail-merge=1 -enable-tail-merge=1", "HOME" => homedir()))
@test !v[1]
@test isempty(v[2])
@test v[3] == "julia: for the --enable-tail-merge option: may only occur zero or one times!"
end
end

let exename = `$(Base.julia_cmd()) --startup-file=no --color=no`
Expand Down