Re: [PATCH] D13351: [Power PC] add soft float support for ppc32

hfinkel added inline comments. Comment at: lib/Driver/Tools.cpp:1477 @@ +1476,3 @@ + // If unspecified, choose the default based on the platform. + if (ABI == ppc::FloatABI::Invalid) { +ABI = ppc::FloatABI::Hard; rjmccall wrote: > hfinkel wrote: > > Don't

Re: [PATCH] D15195: PR4941: Add support for -fno-builtin-foo options.

hfinkel added a subscriber: hfinkel. hfinkel added a comment. Can you use a StringSet instead of a vector and avoid all (most) of the code iterating over the vector of builtins being disabled? Comment at: lib/Frontend/CompilerInvocation.cpp:147 @@ +146,3 @@ +

Re: [PATCH] D14872: PR25575: Make GCC 4.4+ comatible layout for packed bit-fileds of char type

hfinkel added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2783 @@ -2783,1 +2782,3 @@ + "the newer semantic is provided here">, + InGroup>; def

Re: [PATCH] D14872: PR25575: Make GCC 4.4+ comatible layout for packed bit-fileds of char type

hfinkel added a subscriber: hfinkel. hfinkel added a comment. Please upload this patch with full context, see: http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface > Should we add warning about changes in layout for packed bit-fileds of char > type? GCC has it "note:

Re: [PATCH] D13351: [Power PC] add soft float support for ppc32

hfinkel added inline comments. Comment at: lib/CodeGen/TargetInfo.cpp:3421 @@ -3419,3 +3420,3 @@ public: - PPC32_SVR4_ABIInfo(CodeGen::CodeGenTypes ) : DefaultABIInfo(CGT) {} + PPC32_SVR4_ABIInfo(CodeGen::CodeGenTypes , bool SoftFloatABI) : DefaultABIInfo(CGT),

Re: [PATCH] D14871: [Power PC] fix calculating address of arguments on stack for variadic functions

hfinkel added inline comments. Comment at: lib/CodeGen/TargetInfo.cpp:3547 @@ +3546,3 @@ +// Round up address of argument to alignment +llvm::Value *overflow_arg_area = OverflowArea.getPointer(); +uint32_t Align =

Re: [PATCH] D14200: Make FP_CONTRACT ON default.

hfinkel accepted this revision. hfinkel added a comment. This revision is now accepted and ready to land. LGTM, thanks! http://reviews.llvm.org/D14200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D13351: [Power PC] add soft float support for ppc32

hfinkel added a comment. Can you please make sure we produce some sensible error should someone try to use soft float with ppc64 (since we don't support that), and add an associated test. Comment at: lib/Driver/Tools.h:739 @@ +738,3 @@ + Soft, + SoftFP, + Hard,

Re: [PATCH] D11182: [OPENMP 4.0] Initial support for 'omp declare reduction' construct.

hfinkel added inline comments. Comment at: include/clang/Basic/DiagnosticParseKinds.td:995 @@ -994,1 +994,3 @@ +def err_omp_expected_reduction_identifier : Error< + "expected identifier or one of the following operators: '+', '-', '*', '&', '|', '^', '&&' and '||'">;

Re: [PATCH] D13304: Avoid inlining in throw statement

hfinkel added a comment. In http://reviews.llvm.org/D13304#269049, @junbuml wrote: > I just want to ping one more time to see if there is any objection about the > basic idea of this change. If the basic idea itself is acceptable, then I > want to find the best way to get idea in. > > Please

Re: [PATCH] D13304: Avoid inlining in throw statement

hfinkel added a comment. In http://reviews.llvm.org/D13304#276660, @junbuml wrote: > Did you mean to add the Cold in the exception handling region itself instead > of callsite in throw statements ? We already have BranchProbabilityInfo::calcColdCallHeuristics, and so adding it to the

Re: [PATCH] D13351: [Power PC] add soft float support for ppc32

hfinkel added inline comments. Comment at: include/clang/Basic/TargetInfo.h:688 @@ -687,1 +687,3 @@ + virtual bool isSoftFloatABI() const { +return false; Instead of adding this function, please use the same mechanism as X86_32TargetCodeGenInfo and

Re: [PATCH] D12979: Avoid inlining in exception handling context

hfinkel added a comment. You should start a new differential for this, so that you can get a clean summary e-mail with a description sent to cfe-commits. There's some overlap, but you'll also get potentially-different reviewers here. http://reviews.llvm.org/D12979

Re: [PATCH] D12840: [cfe-dev] Enabling ThreadSanitizer on PPC64(BE/LE) plarforms

hfinkel added a subscriber: hfinkel. hfinkel accepted this revision. hfinkel added a reviewer: hfinkel. hfinkel added a comment. This revision is now accepted and ready to land. LGTM, although don't commit until any necessary backend/compiler-rt patches are in. http://reviews.llvm.org/D12840

Re: [PATCH] D11815: Pass subtarget feature "force-align-stack"

hfinkel accepted this revision. hfinkel added a comment. This revision is now accepted and ready to land. In http://reviews.llvm.org/D11815#243031, @hfinkel wrote: > In http://reviews.llvm.org/D11815#242616, @ahatanak wrote: > > > Hal, do you have any thoughts on the points Vasileios brought up?

Re: [PATCH] D11815: Pass subtarget feature "force-align-stack"

hfinkel added a comment. In http://reviews.llvm.org/D11815#242616, @ahatanak wrote: > Hal, do you have any thoughts on the points Vasileios brought up? Currently, > many of the targets don't guarantee that the realigned stack is at least as > aligned as the default alignment required by the

Re: [PATCH] D12313: Introduce __builtin_nontemporal_store and __builtin_nontemporal_load.

hfinkel added a comment. In http://reviews.llvm.org/D12313#241697, @mzolotukhin wrote: > Hi Richard, Hal, > > Does the patch look good now? Looks good to me, but please wait for Richard on the changes he requested. > Thanks, > Michael http://reviews.llvm.org/D12313

Re: [PATCH] D12313: Introduce __builtin_nontemporal_store and __builtin_nontemporal_load.

hfinkel added a comment. Thanks, but I still have this question: Plus, I don't understand why you're implementing another place in CodeGen that generates IR to load and store scalar values. Can't you enhance EmitLoadOfScalar/EmitStoreOfScalar and then use those functions? If nothing else,

Re: [PATCH] D12313: Introduce __builtin_nontemporal_store and __builtin_nontemporal_load.

hfinkel added a comment. I also like this intrinsic approach better than the type attribute. The fact that it does not work with builtin vector types, however, is quite unfortunate. Plus, I don't understand why you're implementing another place in CodeGen that generates IR to load and store

Re: [PATCH] D11815: Pass subtarget feature force-align-stack

hfinkel added inline comments. Comment at: lib/Driver/Tools.cpp:4232 @@ +4231,3 @@ + false)) +CmdArgs.push_back(Args.MakeArgString(-force-align-stack)); + ahatanak wrote: echristo wrote: hfinkel wrote: echristo wrote: hfinkel

Re: [PATCH] D11815: Pass subtarget feature force-align-stack

hfinkel added a subscriber: hfinkel. hfinkel requested changes to this revision. hfinkel added a reviewer: hfinkel. hfinkel added a comment. This revision now requires changes to proceed. As I've said in the review for http://reviews.llvm.org/D11814, this should be added to TargetOptions, and