[PATCH] D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes

Nick Desaulniers via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 31 13:07:50 PDT 2020


nickdesaulniers added a comment.

In D72194#1953100 <https://reviews.llvm.org/D72194#1953100>, @bd1976llvm wrote:

> Split testing of error and output to defeat redirection race.


Heh, I was thinking about this this morning; how could we see output when not redirecting, but not see output when piped (to `less` or `FileCheck`).  The only thing I could think of was a race, or rather forgetting to flush `outs()`, `errs()`, and `dbgs()` prior to exit.  Could this be fixed with a call to `raw_ostream::flush` somewhere? Probably, but that's not your problem to solve (and adding `flush` calls explicitly here would be duct tape for a larger architectural issue in LLVM).

In D72194#1953155 <https://reviews.llvm.org/D72194#1953155>, @bd1976llvm wrote:

> With regards to the diagnostic. The whole thing is a bit of a mess. Since I originally authored this patch the behaviour of report_fatal_error has been changed, see comments on: https://reviews.llvm.org/rG8cedf0e2994c1a258902ed931abdec5f94725a55.


Bad luck.



================
Comment at: llvm/test/CodeGen/X86/explict-section-mergeable.ll:290
+;; Don't create mergeable sections for globals with an explicit section name.
+; RUN: echo '@explicit = unnamed_addr constant [2 x i16] [i16 1, i16 1], section ".explicit"' > %t.no_i_as.ll
+; RUN: llc < %t.no_i_as.ll -mtriple=x86_64 --no-integrated-as 2>&1 \
----------------
Would it be better to make this a separate test file, rather using `echo`, but then passing this file to filecheck?  I think a good general rule of thumb is that `%s` is used as both what generates input to Filecheck over the pipe, AND input to Filecheck itself.  The check doesn't rely on anything else from this file IIUC.  But this is only one test case.  @MaskRay , thoughts?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72194/new/

https://reviews.llvm.org/D72194





More information about the llvm-commits mailing list