Skip to content

Commit

Permalink
Address issue 4369 "as best we can" (#4441)
Browse files Browse the repository at this point in the history
After extensive investigation of the weirdness that is #4369, Joe
and I believe the issue is most likely an LLVM bug. However, we
don't have time to pursue further.

During the investigation, I came to believe that the change to use
CodeGenOpt::Default instead of CodeGenOpt::None for target optimizations
would be prudent.

It prevents the known instances of bugs like #4369 which is nice but
not key to this change. Changes things because they make symptoms
go away isn't a good strategy if you don't understand why. We do not
"fully" understand the impact of this change on bugs like #4369. However,
it appears that CodeGenOpt::None is a code path that sees little use
or testing and is more prone to bugs/regression.

Given that we have seen more than 1 bug that "smells like" #4369, we felt
it was wise to switch to Default from None at least until such time as we
understand #4369 and #3874. LLVM is a complicated beast and using code paths
that are little used can be a dangerous proposition.

This commit doesn't include release notes as we addresses to user facing
instance of #4369 already.
  • Loading branch information
SeanTAllen committed Sep 12, 2023
1 parent 05c75da commit 9d7360b
Showing 1 changed file with 12 additions and 4 deletions.
16 changes: 12 additions & 4 deletions src/libponyc/codegen/host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,20 @@ LLVMTargetMachineRef codegen_machine(LLVMTargetRef target, pass_opt_t* opt)
if(opt->pic)
reloc = Reloc::PIC_;

// The Arm debug fix is a "temporary" fix for issue #3874
// The Arm debug fix is a "temporary" fix for issues #3874 and #4369.
// https://github.com/ponylang/ponyc/issues/3874
// Hopefully we get #3874 figured out in a reasonable amount of time.
// https://github.com/ponylang/ponyc/issues/4369
// We believe both issues are LLVM issues (probably the same or related).
// We invested considerable time in trying to track down "the root cause" but
// haven't been able to.
// Ideally, debug builds would get CodeGenOpt::None here, however, that when
// mixed with other optimization options elsewhere seems to lead to bugs that
// related to DWARF info and inlining.
// As part of the the #4369 investigation, we came to believe that
// CodeGenOpt::None is an infrequently used code path and that we are better
// using Default as it is less likely to have bugs.
CodeGenOpt::Level opt_level =
opt->release ? CodeGenOpt::Aggressive :
target_is_arm(opt->triple) ? CodeGenOpt::Default : CodeGenOpt::None;
opt->release ? CodeGenOpt::Aggressive : CodeGenOpt::Default;

TargetOptions options;

Expand Down

0 comments on commit 9d7360b

Please sign in to comment.