Page MenuHomePhabricator

[NFCi] Switch ordering of ParseLangArgs and ParseCodeGenArgs.
Needs ReviewPublic

Authored by plotfi on Mon, May 11, 10:56 AM.

Details

Reviewers
rjmccall
ab
Summary

After speaking with @ab and @rjmccall on https://reviews.llvm.org/D72841#2027740 and https://github.com/apple/llvm-project/pull/1202.

it sounds like code-gen option parsing should depend on language-option parsing, not the other way around. Apple Clang llvm-project master does this in the right order but upstream llvm.org does it in the opposite order.

This patch changes the order to the way apple-master does it, which we think is the correct order. At the moment D72841 causes some bot failures on apple-master (only on tests added in D72841). I think D72841 should be reverted, and then reapplied after _this_ patch is landed.

Diff Detail

Unit TestsFailed

TimeTest
110 msClang.CodeGen::finite-math.c
Script: -- : 'RUN: at line 1'; c:\ws\prod\llvm-project\build\bin\clang.exe -cc1 -internal-isystem c:\ws\prod\llvm-project\build\lib\clang\11.0.0\include -nostdsysteminc -ffinite-math-only -emit-llvm -o - C:\ws\prod\llvm-project\clang\test\CodeGen\finite-math.c | c:\ws\prod\llvm-project\build\bin\filecheck.exe C:\ws\prod\llvm-project\clang\test\CodeGen\finite-math.c -check-prefix=CHECK -check-prefix=FINITE
340 msClang.CodeGen::fp-floatcontrol-stack.cpp
Script: -- : 'RUN: at line 1'; c:\ws\prod\llvm-project\build\bin\clang.exe -cc1 -internal-isystem c:\ws\prod\llvm-project\build\lib\clang\11.0.0\include -nostdsysteminc -triple x86_64-linux-gnu -ffp-contract=on -DDEFAULT=1 -emit-llvm -o - C:\ws\prod\llvm-project\clang\test\CodeGen\fp-floatcontrol-stack.cpp | c:\ws\prod\llvm-project\build\bin\filecheck.exe --check-prefix=CHECK-DDEFAULT C:\ws\prod\llvm-project\clang\test\CodeGen\fp-floatcontrol-stack.cpp
300 msClang.CodeGenOpenCL::relaxed-fpmath.cl
Script: -- : 'RUN: at line 1'; c:\ws\prod\llvm-project\build\bin\clang.exe -cc1 -internal-isystem c:\ws\prod\llvm-project\build\lib\clang\11.0.0\include -nostdsysteminc C:\ws\prod\llvm-project\clang\test\CodeGenOpenCL\relaxed-fpmath.cl -emit-llvm -o - | c:\ws\prod\llvm-project\build\bin\filecheck.exe C:\ws\prod\llvm-project\clang\test\CodeGenOpenCL\relaxed-fpmath.cl -check-prefix=NORMAL
80 msClang.Frontend::diagnostics-order.c
Script: -- : 'RUN: at line 5'; not c:\ws\prod\llvm-project\build\bin\clang.exe -cc1 -internal-isystem c:\ws\prod\llvm-project\build\lib\clang\11.0.0\include -nostdsysteminc -O999 -std=bogus -verify=-foo C:\ws\prod\llvm-project\clang\test\Frontend\diagnostics-order.c 2> C:\ws\prod\llvm-project\build\tools\clang\test\Frontend\Output\diagnostics-order.c.tmp

Event Timeline

plotfi created this revision.Mon, May 11, 10:56 AM
Herald added a project: Restricted Project. · View Herald TranscriptMon, May 11, 10:56 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

I don't think the reversion is necessary; it's being fixed to remove the dependency. This is a good change, though.

I don't think the reversion is necessary; it's being fixed to remove the dependency. This is a good change, though.

Should I still try to land this diff so that it matches the behavior on Apple master? Might catch any future dependency issues.

I think it makes sense; if nothing else, we're trying to upstream all that work.