eliminate code related to --no-ir-gen

eliminate code related to --no-ir-gen

BUG=
R=stichnot@chromium.org

Committed: https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a=commit;h=0d4fc92b7a200524e6e1e49002fdde58438a1cfe

[rkotlerimgtec, 2015-12-12 03:07:08]

Description was changed from

==========
eliminate code related to --no-ir-gen

BUG=
==========

to

==========
eliminate code related to --no-ir-gen

BUG=
==========

[rkotlerimgtec, 2015-12-12 03:07:08]

rkotlerimgtec@gmail.com changed reviewers:
+ jpp@chromium.org, kschimpf@chromium.org, stichnot@chromium.org

[rkotlerimgtec, 2015-12-12 03:08:30]

Checked this by inspection and ran make check.

Have not so far run full tests for putbacks so was not sure exactly how to do
that here. Will look into that but before LGTM I suggest you run everything
since this patch touches a lot of code.

[Jim Stichnoth, 2015-12-12 04:11:02]

Once you make these further changes, you also need to remove -no-ir-gen from the
various lit tests.  This command should work:

find tests_lit/ -type f | xargs sed -i 's/ -no-ir-gen//'

https://codereview.chromium.org/1522433004/diff/1/src/IceClFlags.cpp
File src/IceClFlags.cpp (right):

https://codereview.chromium.org/1522433004/diff/1/src/IceClFlags.cpp#newcode86
src/IceClFlags.cpp:86: cl::opt<bool> DisableIRGeneration("no-ir-gen",
remove this

https://codereview.chromium.org/1522433004/diff/1/src/IceClFlags.cpp#newcode393
src/IceClFlags.cpp:393: OutFlags.DisableIRGeneration = false;
remove this

https://codereview.chromium.org/1522433004/diff/1/src/IceClFlags.cpp#newcode437
src/IceClFlags.cpp:437: if (::DisableIRGeneration)
remove this

https://codereview.chromium.org/1522433004/diff/1/src/IceClFlags.h
File src/IceClFlags.h (right):

https://codereview.chromium.org/1522433004/diff/1/src/IceClFlags.h#newcode258
src/IceClFlags.h:258: bool DisableIRGeneration;
remove this

https://codereview.chromium.org/1522433004/diff/1/src/PNaClTranslator.cpp
File src/PNaClTranslator.cpp (right):

https://codereview.chromium.org/1522433004/diff/1/src/PNaClTranslator.cpp#new...
src/PNaClTranslator.cpp:533: C = getConstantSym(Func->getName(),
Func->getSuppressMangling(),
Combine this with the previous statement.

https://codereview.chromium.org/1522433004/diff/1/src/PNaClTranslator.cpp#new...
src/PNaClTranslator.cpp:546: C = getConstantSym(Decl->getName(),
Decl->getSuppressMangling(),
Combine this with the previous statement.

[Jim Stichnoth, 2015-12-12 04:14:01]

https://codereview.chromium.org/1522433004/diff/20001/Makefile
File Makefile (right):

https://codereview.chromium.org/1522433004/diff/20001/Makefile#newcode24
Makefile:24: -DALLOW_LLVM_IR_AS_INPUT=0 \
Do the same for Makefile.standalone and CMakeLists.txt.

Also, probably nice if you reflow the lines.

[rkotlerimgtec, 2015-12-12 05:33:03]

https://codereview.chromium.org/1522433004/diff/1/src/IceClFlags.cpp
File src/IceClFlags.cpp (right):

https://codereview.chromium.org/1522433004/diff/1/src/IceClFlags.cpp#newcode86
src/IceClFlags.cpp:86: cl::opt<bool> DisableIRGeneration("no-ir-gen",
On 2015/12/12 04:11:02, stichnot wrote:
> remove this

Done.

https://codereview.chromium.org/1522433004/diff/1/src/IceClFlags.cpp#newcode393
src/IceClFlags.cpp:393: OutFlags.DisableIRGeneration = false;
On 2015/12/12 04:11:02, stichnot wrote:
> remove this

Done.

https://codereview.chromium.org/1522433004/diff/1/src/IceClFlags.cpp#newcode437
src/IceClFlags.cpp:437: if (::DisableIRGeneration)
On 2015/12/12 04:11:02, stichnot wrote:
> remove this

Done.

https://codereview.chromium.org/1522433004/diff/1/src/IceClFlags.h
File src/IceClFlags.h (right):

https://codereview.chromium.org/1522433004/diff/1/src/IceClFlags.h#newcode258
src/IceClFlags.h:258: bool DisableIRGeneration;
On 2015/12/12 04:11:02, stichnot wrote:
> remove this

Done.

https://codereview.chromium.org/1522433004/diff/1/src/PNaClTranslator.cpp
File src/PNaClTranslator.cpp (right):

https://codereview.chromium.org/1522433004/diff/1/src/PNaClTranslator.cpp#new...
src/PNaClTranslator.cpp:533: C = getConstantSym(Func->getName(),
Func->getSuppressMangling(),
On 2015/12/12 04:11:02, stichnot wrote:
> Combine this with the previous statement.

Done.

https://codereview.chromium.org/1522433004/diff/1/src/PNaClTranslator.cpp#new...
src/PNaClTranslator.cpp:546: C = getConstantSym(Decl->getName(),
Decl->getSuppressMangling(),
On 2015/12/12 04:11:02, stichnot wrote:
> Combine this with the previous statement.

Done.

https://codereview.chromium.org/1522433004/diff/20001/Makefile
File Makefile (right):

https://codereview.chromium.org/1522433004/diff/20001/Makefile#newcode24
Makefile:24: -DALLOW_LLVM_IR_AS_INPUT=0 \
On 2015/12/12 04:14:01, stichnot wrote:
> Do the same for Makefile.standalone and CMakeLists.txt.
> 
> Also, probably nice if you reflow the lines.

Done.

[Jim Stichnoth, 2015-12-12 15:43:42]

https://codereview.chromium.org/1522433004/diff/40001/src/IceBuildDefs.h
File src/IceBuildDefs.h (left):

https://codereview.chromium.org/1522433004/diff/40001/src/IceBuildDefs.h#oldc...
src/IceBuildDefs.h:22: constexpr bool disableIrGen() { return
ALLOW_DISABLE_IR_GEN; }
Rebase from master and fix resulting conflicts. :(

  git pull --rebase
  (fix conflicts manually)
  git add src/IceBuildDefs.h
  git rebase --continue

https://codereview.chromium.org/1522433004/diff/40001/src/IceClFlags.cpp
File src/IceClFlags.cpp (right):

https://codereview.chromium.org/1522433004/diff/40001/src/IceClFlags.cpp#newc...
src/IceClFlags.cpp:434: 
remove extra blank line as well

https://codereview.chromium.org/1522433004/diff/40001/tests_lit/reader_tests/...
File tests_lit/reader_tests/alloca.ll (right):

https://codereview.chromium.org/1522433004/diff/40001/tests_lit/reader_tests/...
tests_lit/reader_tests/alloca.ll:4: ; RUN: %if --need=allow_disable_ir_gen
--command \
Sorry, I missed a key part.

The parts that produce a fake success when the allow_diable_ir_gen attribute is
missing, also need to be removed.

You can start with this script:

find tests_lit/ -type f | xargs sed -i 's/%if --need=allow_disable_ir_gen
--command //'

and then manually fix up the RUN lines that are empty or just contain "|" .

[Jim Stichnoth, 2015-12-13 15:55:20]

https://codereview.chromium.org/1522433004/diff/40001/src/IceBuildDefs.h
File src/IceBuildDefs.h (left):

https://codereview.chromium.org/1522433004/diff/40001/src/IceBuildDefs.h#oldc...
src/IceBuildDefs.h:22: constexpr bool disableIrGen() { return
ALLOW_DISABLE_IR_GEN; }
On 2015/12/12 15:43:42, stichnot wrote:
> Rebase from master and fix resulting conflicts. :(
> 
>   git pull --rebase
>   (fix conflicts manually)
>   git add src/IceBuildDefs.h
>   git rebase --continue

BTW, for me "make format" is reformatting the version of IceBuildDefs.h in
master, so I guess we forgot to reformat before that last commit?  In any case,
we should take care of that in this CL.

[rkotlerimgtec, 2015-12-14 02:23:02]

I'm almost done making these changes. In case you are online now, do we
really need these
--check-prefix=NOIR tests anymoe?

On Sun, Dec 13, 2015 at 7:55 AM, <stichnot@chromium.org> wrote:

>
> https://codereview.chromium.org/1522433004/diff/40001/src/IceBuildDefs.h
> File src/IceBuildDefs.h (left):
>
>
>
https://codereview.chromium.org/1522433004/diff/40001/src/IceBuildDefs.h#oldc...
> src/IceBuildDefs.h:22: constexpr bool disableIrGen() { return
> ALLOW_DISABLE_IR_GEN; }
> On 2015/12/12 15:43:42, stichnot wrote:
>
>> Rebase from master and fix resulting conflicts. :(
>>
>
>    git pull --rebase
>>    (fix conflicts manually)
>>    git add src/IceBuildDefs.h
>>    git rebase --continue
>>
>
> BTW, for me "make format" is reformatting the version of IceBuildDefs.h
> in master, so I guess we forgot to reformat before that last commit?  In
> any case, we should take care of that in this CL.
>
> https://codereview.chromium.org/1522433004/
>

-- 
You received this message because you are subscribed to the Google Groups
"Native-Client-Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to native-client-reviews+unsubscribe@googlegroups.com.
To post to this group, send email to native-client-reviews@googlegroups.com.
Visit this group at https://groups.google.com/group/native-client-reviews.
For more options, visit https://groups.google.com/d/optout.

[rkotlerimgtec, 2015-12-14 03:12:12]

https://codereview.chromium.org/1522433004/diff/40001/src/IceBuildDefs.h
File src/IceBuildDefs.h (left):

https://codereview.chromium.org/1522433004/diff/40001/src/IceBuildDefs.h#oldc...
src/IceBuildDefs.h:22: constexpr bool disableIrGen() { return
ALLOW_DISABLE_IR_GEN; }
On 2015/12/12 15:43:42, stichnot wrote:
> Rebase from master and fix resulting conflicts. :(
> 
>   git pull --rebase
>   (fix conflicts manually)
>   git add src/IceBuildDefs.h
>   git rebase --continue

Done.

https://codereview.chromium.org/1522433004/diff/40001/src/IceBuildDefs.h#oldc...
src/IceBuildDefs.h:22: constexpr bool disableIrGen() { return
ALLOW_DISABLE_IR_GEN; }
On 2015/12/13 15:55:20, stichnot wrote:
> On 2015/12/12 15:43:42, stichnot wrote:
> > Rebase from master and fix resulting conflicts. :(
> > 
> >   git pull --rebase
> >   (fix conflicts manually)
> >   git add src/IceBuildDefs.h
> >   git rebase --continue
> 
> BTW, for me "make format" is reformatting the version of IceBuildDefs.h in
> master, so I guess we forgot to reformat before that last commit?  In any
case,
> we should take care of that in this CL.

Done.

https://codereview.chromium.org/1522433004/diff/40001/src/IceClFlags.cpp
File src/IceClFlags.cpp (right):

https://codereview.chromium.org/1522433004/diff/40001/src/IceClFlags.cpp#newc...
src/IceClFlags.cpp:434: 
On 2015/12/12 15:43:42, stichnot wrote:
> remove extra blank line as well

Done.

https://codereview.chromium.org/1522433004/diff/40001/tests_lit/reader_tests/...
File tests_lit/reader_tests/alloca.ll (right):

https://codereview.chromium.org/1522433004/diff/40001/tests_lit/reader_tests/...
tests_lit/reader_tests/alloca.ll:4: ; RUN: %if --need=allow_disable_ir_gen
--command \
On 2015/12/12 15:43:42, stichnot wrote:
> Sorry, I missed a key part.
> 
> The parts that produce a fake success when the allow_diable_ir_gen attribute
is
> missing, also need to be removed.
> 
> You can start with this script:
> 
> find tests_lit/ -type f | xargs sed -i 's/%if --need=allow_disable_ir_gen
> --command //'
> 
> and then manually fix up the RUN lines that are empty or just contain "|" .

Done.

[Jim Stichnoth, 2015-12-14 05:09:45]

On 2015/12/14 02:23:02, rkotlerimgtec wrote:
> I'm almost done making these changes. In case you are online now, do we
> really need these
> --check-prefix=NOIR tests anymoe?

I guess that prefix is more about -notranslate, so I think it's still valid even
without -no-ir-gen.

That style of test was introduced in https://codereview.chromium.org/696383004/
, maybe Karl might remember more about the circumstances.

[Jim Stichnoth, 2015-12-14 05:36:13]

lgtm

[Jim Stichnoth, 2015-12-14 05:36:37]

Description was changed from

==========
eliminate code related to --no-ir-gen

BUG=
==========

to

==========
eliminate code related to --no-ir-gen

BUG=
R=stichnot@chromium.org

Committed:
https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a...
==========

[Jim Stichnoth, 2015-12-14 05:36:38]

Committed patchset #4 (id:60001) manually as
0d4fc92b7a200524e6e1e49002fdde58438a1cfe (presubmit successful).