Adds accessor methods to class ClFlags.

Adds accessor methods to class ClFlags.

Allows one to define explicit overrides in get accessors, based on
compilation features. To show usage, modified SubConstantCalls to
never be enabled if building a minimal llvm2ice.

BUG=None
R=stichnot@chromium.org

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

[Karl, 2015-02-05 21:56:12]

kschimpf@google.com changed reviewers:
+ jvoung@chromium.org, stichnot@chromium.org

[Jim Stichnoth, 2015-02-05 22:30:09]

https://codereview.chromium.org/905463003/diff/1/Makefile.standalone
File Makefile.standalone (right):

https://codereview.chromium.org/905463003/diff/1/Makefile.standalone#newcode57
Makefile.standalone:57: -DALLOW_LLVM_IR_AS_INPUT=0 -DALLOW_DISABLE_IR_GEN=0
-DIS_MINIMAL_BUILD=1
In the interest of bikeshedding, can we have this define begin with ALLOW_ to
keep the pattern easily recognizable in the code?

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

https://codereview.chromium.org/905463003/diff/1/src/IceClFlags.h#newcode21
src/IceClFlags.h:21: class ClFlags {
Since this is becoming a proper class, disable default copy ctor and assignment
operator.

https://codereview.chromium.org/905463003/diff/1/src/IceClFlags.h#newcode89
src/IceClFlags.h:89: return IS_MINIMAL_BUILD ? false : StubConstantCalls;
Stylistically, I'd prefer:
  return !IS_MINIMAL_BUILD && StubConstantCalls;
which is a closer match to the uses of ALLOW_ in the rest of the code.

https://codereview.chromium.org/905463003/diff/1/src/IceClFlags.h#newcode129
src/IceClFlags.h:129: bool DisableInternal;
While you're here, could you alphabetically sort the field names (and accessor
functions) within each type?

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

https://codereview.chromium.org/905463003/diff/1/src/PNaClTranslator.cpp#newc...
src/PNaClTranslator.cpp:194: return ALLOW_DISABLE_IR_GEN ?
Translator.getFlags().getDisableIRGeneration()
Change to
  return ALLOW_DISABLE_IR_GEN &&
getTranslator().getFlags().getDisableIRGeneration();

Or better yet, change getDisableIRGeneration() to
  return ALLOW_DISABLE_IR_GEN && DisableIRGeneration;
and simplify the two uses in this file and the one in IceGlobalContext.h.

[Karl, 2015-02-05 23:57:39]

https://codereview.chromium.org/905463003/diff/1/Makefile.standalone
File Makefile.standalone (right):

https://codereview.chromium.org/905463003/diff/1/Makefile.standalone#newcode57
Makefile.standalone:57: -DALLOW_LLVM_IR_AS_INPUT=0 -DALLOW_DISABLE_IR_GEN=0
-DIS_MINIMAL_BUILD=1
On 2015/02/05 22:30:09, stichnot wrote:
> In the interest of bikeshedding, can we have this define begin with ALLOW_ to
> keep the pattern easily recognizable in the code?

Done.

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

https://codereview.chromium.org/905463003/diff/1/src/IceClFlags.h#newcode21
src/IceClFlags.h:21: class ClFlags {
On 2015/02/05 22:30:09, stichnot wrote:
> Since this is becoming a proper class, disable default copy ctor and
assignment
> operator.

Done.

https://codereview.chromium.org/905463003/diff/1/src/IceClFlags.h#newcode89
src/IceClFlags.h:89: return IS_MINIMAL_BUILD ? false : StubConstantCalls;
On 2015/02/05 22:30:09, stichnot wrote:
> Stylistically, I'd prefer:
>   return !IS_MINIMAL_BUILD && StubConstantCalls;
> which is a closer match to the uses of ALLOW_ in the rest of the code.

Done.

https://codereview.chromium.org/905463003/diff/1/src/IceClFlags.h#newcode129
src/IceClFlags.h:129: bool DisableInternal;
On 2015/02/05 22:30:09, stichnot wrote:
> While you're here, could you alphabetically sort the field names (and accessor
> functions) within each type?

Done.

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

https://codereview.chromium.org/905463003/diff/1/src/PNaClTranslator.cpp#newc...
src/PNaClTranslator.cpp:194: return ALLOW_DISABLE_IR_GEN ?
Translator.getFlags().getDisableIRGeneration()
On 2015/02/05 22:30:09, stichnot wrote:
> Change to
>   return ALLOW_DISABLE_IR_GEN &&
> getTranslator().getFlags().getDisableIRGeneration();
> 
> Or better yet, change getDisableIRGeneration() to
>   return ALLOW_DISABLE_IR_GEN && DisableIRGeneration;
> and simplify the two uses in this file and the one in IceGlobalContext.h.

Done. Also did similar changes to: getDumpStats, getGenerateUnitTestMessages,
and getTimeEachFunction.

[Jim Stichnoth, 2015-02-06 13:16:08]

Otherwise LGTM.  Thanks!

https://codereview.chromium.org/905463003/diff/20001/src/IceClFlags.h
File src/IceClFlags.h (right):

https://codereview.chromium.org/905463003/diff/20001/src/IceClFlags.h#newcode171
src/IceClFlags.h:171: IceString DefaultFunctionPrefix;
Put DefaultFunctionPrefix before DefaultGlobalPrefix

https://codereview.chromium.org/905463003/diff/20001/src/PNaClTranslator.cpp
File src/PNaClTranslator.cpp (right):

https://codereview.chromium.org/905463003/diff/20001/src/PNaClTranslator.cpp#...
src/PNaClTranslator.cpp:633: if (!getFlags().getGenerateUnitTestMessages()) {
Change "if (!c) a; else b;" pattern to "if (c) b; else a;"

[Karl, 2015-02-09 17:38:34]

New patchsets have been uploaded after l-g-t-m from stichnot@chromium.org

[Karl, 2015-02-09 22:20:31]

Committed patchset #3 (id:40001) manually as
df80eb862a6e44a4ee0a4e868f5ef1543f368020 (presubmit successful).

[Karl, 2015-02-09 22:20:44]

https://codereview.chromium.org/905463003/diff/20001/src/IceClFlags.h
File src/IceClFlags.h (right):

https://codereview.chromium.org/905463003/diff/20001/src/IceClFlags.h#newcode171
src/IceClFlags.h:171: IceString DefaultFunctionPrefix;
On 2015/02/06 13:16:08, stichnot wrote:
> Put DefaultFunctionPrefix before DefaultGlobalPrefix

Done.

https://codereview.chromium.org/905463003/diff/20001/src/PNaClTranslator.cpp
File src/PNaClTranslator.cpp (right):

https://codereview.chromium.org/905463003/diff/20001/src/PNaClTranslator.cpp#...
src/PNaClTranslator.cpp:633: if (!getFlags().getGenerateUnitTestMessages()) {
On 2015/02/06 13:16:08, stichnot wrote:
> Change "if (!c) a; else b;" pattern to "if (c) b; else a;"

Done.