[PATCH] D42720: [CodeGen] Switch non-SJLJ EH encoding to uleb128

Rafael Avila de Espindola via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 6 08:26:00 PST 2018


Ryan Prichard via Phabricator via llvm-commits
<llvm-commits at lists.llvm.org> writes:


> Index: lib/MC/MCObjectStreamer.cpp
> ===================================================================
> --- lib/MC/MCObjectStreamer.cpp
> +++ lib/MC/MCObjectStreamer.cpp
> @@ -51,17 +51,36 @@
>    PendingLabels.clear();
>  }
>  
> +static Optional<uint64_t> absoluteSymbolDiff(const MCSymbol *Hi,
> +                                             const MCSymbol *Lo) {
> +  if (!Hi->getFragment() || Hi->getFragment() != Lo->getFragment() ||
> +      Hi->isVariable() || Lo->isVariable())
> +    return None;
> +
> +  return Hi->getOffset() - Lo->getOffset();
> +}

Add a comment saying this is just an optimization to avoid creating a
fragment when the final value is known.

>  void MCObjectStreamer::emitAbsoluteSymbolDiff(const MCSymbol *Hi,
>                                                const MCSymbol *Lo,
>                                                unsigned Size) {
> -  // If not assigned to the same (valid) fragment, fallback.
> -  if (!Hi->getFragment() || Hi->getFragment() != Lo->getFragment() ||
> -      Hi->isVariable() || Lo->isVariable()) {
> +  Optional<uint64_t> Diff = absoluteSymbolDiff(Hi, Lo);
> +  if (!Diff.hasValue()) {
>      MCStreamer::emitAbsoluteSymbolDiff(Hi, Lo, Size);
>      return;
>    }
>  
> -  EmitIntValue(Hi->getOffset() - Lo->getOffset(), Size);
> +  EmitIntValue(*Diff, Size);


I think you can write this as

if (Optional<uint64_t> Diff = ...) {
  EmitIntValue(*Diff, Size);
  return;
}
MCStreamer::emitAbsoluteSymbolDiff(Hi, Lo, Size);

This patch does two things:

* Remove the manual accounting and replace it with using assembler
  expressions.
* Use uleb.

The first part should be an awesome nop cleanup. Could it be split out
of this patch?

Thanks,
Rafael


More information about the llvm-commits mailing list