Skip to content

Commit

Permalink
Address issue 4369 "as best we can"
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 a7c5e42
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 a7c5e42

Please sign in to comment.