DavidKreitzer accepted D47182: [X86] Move all Intel defined intrinsic includes into immintrin.h.
DavidKreitzer added a comment to D47182: [X86] Move all Intel defined intrinsic includes into immintrin.h.
Hi Craig, just one comment on the details. Everything else looks good.
I agree with the changes in x86intrin.h and immintrin.h. For the others, I question whether we ought to recommend inclusion of x86intrin.h or immintrin.h. The distinction as I understand it is that immintrin.h is used for Intel-specific intrinsics while x86intrin.h covers all intrinsics for x86-based architectures.
DavidKreitzer added a comment to D47174: [X86] Move 128-bit f16c intrinsics to __emmintrin_f16c.h include from emmintrin.h. Move 256-bit f16c intrinsics back to f16cintrin.h.
DavidKreitzer accepted D46863: [X86] Use __builtin_convertvector to implement some of the packed integer to packed float conversion intrinsics..
I had actually done that as part of my initial review, so LGTM.
DavidKreitzer added a comment to D47029: [X86] Remove some preprocessor feature checks from intrinsic headers.
Looks right to me, Craig.
DavidKreitzer added a comment to D46863: [X86] Use __builtin_convertvector to implement some of the packed integer to packed float conversion intrinsics..
This looks good to me, Craig. I am not worried about the constant folding issue, as I think constant folding these conversion intrinsics (assuming round-to-nearest) is a perfectly valid optimization in the absence of FENV_ACCESS. (FWIW, we don't do this constant folding in icc, but that is only because we have never gotten around to implementing it.) I am also not worried about the spurious 'inexact' exceptions that the changes to the mask intrinsics will cause.
DavidKreitzer accepted D43441: [X86][AVX512DQ] Use packed instructions for scalar FP<->i64 conversions on 32-bit targets (PR31630).
DavidKreitzer added a comment to D43441: [X86][AVX512DQ] Use packed instructions for scalar FP<->i64 conversions on 32-bit targets (PR31630).
DavidKreitzer added a comment to D44583: Quiet unused variable warnings in Release builds.
Thanks, Craig. I noticed it with gcc 4.8.5.
DavidKreitzer added a reviewer for D44583: Quiet unused variable warnings in Release builds: jwroorda.
DavidKreitzer created D44583: Quiet unused variable warnings in Release builds.
DavidKreitzer added a comment to D40975: [LangRef] Reflect the changes in the TBAA info format.
This looks like a nice change.
DavidKreitzer added a comment to D39575: [X86] Add subtarget features prefer-avx256 and prefer-avx128 and use them to limit vector width presented by TTI.
DavidKreitzer added inline comments to D38730: X86: Fix X86CallFrameOptimization to search for the COPY StackPointer.
DavidKreitzer added inline comments to D38738: X86CallFrameOptimization: Recognize 'store 0/-1 using and/or' idioms.
DavidKreitzer added a comment to D38730: X86: Fix X86CallFrameOptimization to search for the COPY StackPointer.
DavidKreitzer accepted D38729: X86: Register the X86CallFrameOptimization pass.
DavidKreitzer added a comment to D37289: [X86] Speculatively load operands of select instruction.
After some internal discussions, we suspect that Sanjay's counter-example may have undefined behavior according to the C standard.
DavidKreitzer accepted D38001: [X86] Don't select anyext GR32->GR64 to SUBREG_TO_REG. Use INSERT_SUBREG instead..
I'd recommend going ahead with this, Craig. Even if it isn't possible for this bug to trigger a failure today, it looks like a latent bug that should be fixed.
DavidKreitzer added a comment to D38001: [X86] Don't select anyext GR32->GR64 to SUBREG_TO_REG. Use INSERT_SUBREG instead..
This change seems obviously correct. The diffs in the tests all look like tweaks in the register allocation behavior and aren't testing the behavior you are fixing. Is it possible to write a test that miscompiles due to the existing bug?
DavidKreitzer accepted D37729: [X86] Make sure we still emit zext for GR32 to GR64 when the source of the zext is AssertZext.
DavidKreitzer added reviewers for D37559: [X86FixupBWInsts] More presise register liveness if no <imp-use> on MOVs.: andrew.w.kaylor, craig.topper.
DavidKreitzer added inline comments to D37510: Unsigned saturation subtraction canonicalization.
DavidKreitzer accepted D36344: [X86] Don't use fsin/fcos/fsincos instructions ever.
DavidKreitzer added a comment to D36414: [X86][X87] Ensure x87 instructions are tagged as altering the FPSW reg.
DavidKreitzer added a comment to D36344: [X86] Don't use fsin/fcos/fsincos instructions ever.
DavidKreitzer requested changes to D36036: Supported interleaved byte load-pattern of stride:4 VF(8, 16, 32)..
DavidKreitzer added inline comments to D35829: [X86][LLVM]Expanding Supports lowerInterleavedStore() in X86InterleavedAccess (VF16 stride 4)..
DavidKreitzer accepted D34601: [X86][LLVM]Expanding Supports lowerInterleavedStore() in X86InterleavedAccess..
DavidKreitzer added a comment to D34601: [X86][LLVM]Expanding Supports lowerInterleavedStore() in X86InterleavedAccess..
DavidKreitzer added inline comments to D34601: [X86][LLVM]Expanding Supports lowerInterleavedStore() in X86InterleavedAccess..
DavidKreitzer accepted D35638: A fix for bug33826.
DavidKreitzer added inline comments to D35638: A fix for bug33826.
DavidKreitzer accepted D32658: Supports lowerInterleavedStore() in X86InterleavedAccess..
DavidKreitzer added a comment to D32658: Supports lowerInterleavedStore() in X86InterleavedAccess..
Thanks for the fixes, Farhana. Other than a couple typos, this looks good.
DavidKreitzer added a comment to D33684: Added tests for lowerInterleavedStore..
Thanks, Farhana. Please remember to add llvm-commits as a subscriber for LLVM patches. This LGTM unless Simon has additional suggestions.
DavidKreitzer added inline comments to D32658: Supports lowerInterleavedStore() in X86InterleavedAccess..
DavidKreitzer added inline comments to D32669: [Nios2] Target registration.
DavidKreitzer added a comment to D30049: x86 interrupt calling convention: re-align stack pointer on 64-bit if an error code was pushed.
DavidKreitzer abandoned D21479: Prototype fix for replacing the SETcc/MOVZX idiom with XOR/SETcc for a 32-bit conditional set of 0,1.
DavidKreitzer added a comment to D27028: Add intrinsics for constrained floating point operations.
When frem has a meaningful result, it is always exact, so perhaps we should omit the rounding behavior argument? I think we still need a constrained frem intrinsic, though, to handle the exceptional behavior in cases such as "frem inf, x" and "frem x, 0".
The implementation is slightly simpler if I give the frem intrinsic a rounding argument, even though it isn't needed. Otherwise, that intrinsic couldn't be handled by the same subclass as the others. I realize that's a fairly weak argument, but I just feel like making this one intrinsic different from the others will make the code ugly.
DavidKreitzer abandoned D27319: Fix buildbot failure in non-assert-enabled -Werror build due to r288410.
Looks like this was fixed by r288416.
DavidKreitzer retitled D27319: Fix buildbot failure in non-assert-enabled -Werror build due to r288410 from to Fix buildbot failure in non-assert-enabled -Werror build due to r288410.
DavidKreitzer added a comment to D25986: Re-factorization of X86InterleaveAccess into a class.
I have no further comments. This LGTM too.
DavidKreitzer added inline comments to D25986: Re-factorization of X86InterleaveAccess into a class.
DavidKreitzer added a comment to D25350: [X86] Enable interleaved memory accesses by default.
DavidKreitzer added a comment to D25566: Fix for PR30687. Avoid dereferencing MBB.end()..
Thanks for taking care of this, Reid! That fix LGTM.
DavidKreitzer retitled D25566: Fix for PR30687. Avoid dereferencing MBB.end(). from to Fix for PR30687. Avoid dereferencing MBB.end()..
DavidKreitzer accepted D24681: Optimize patterns of vectorized interleaved memory accesses for X86..
DavidKreitzer added a comment to D24681: Optimize patterns of vectorized interleaved memory accesses for X86..
DavidKreitzer added inline comments to D24681: Optimize patterns of vectorized interleaved memory accesses for X86..
DavidKreitzer requested changes to D24681: Optimize patterns of vectorized interleaved memory accesses for X86..
DavidKreitzer added a comment to D23253: [X86] Generalized transformation of `definstr gr8; movzx gr32, gr8` to `xor gr32, gr32; definstr gr8`.
DavidKreitzer retitled D22941: Fix confusion over x86_64 CMOV semantics in order to avoid unnecessary zero extensions from to Fix confusion over x86_64 CMOV semantics in order to avoid unnecessary zero extensions.
DavidKreitzer added a comment to D22045: [X86] Support of no_caller_saved_registers attribute (Clang part).
DavidKreitzer added a comment to D21774: [X86] Transform setcc + movzbl into xorl + setcc.
I have no objection to this solution. I think it is robust in a functional sense. And we can always change it later if we discover that the worst case MOV32r0 sinking scenario is more common than we think. It would give me the warm fuzzies if we had some experimental evidence to confirm the suspicion that this is a rare case. Do you already have that? If not, maybe it would be a good idea to write a late pass that looks for this kind of pattern and count the number of occurrences on, say, cpu2006?
And thanks for the performance data! Once this gets committed, I'll have someone run testing on a broader set of workloads.
DavidKreitzer added a comment to D21560: Relax the clearance calculating for breaking partial register dependency..
I wasn't suggesting that Marina's fixes should replace this one. Sorry for that confusion. My point was simply that this is just one piece of a more comprehensive solution to the false dependence problem.
DavidKreitzer added a reviewer for D21560: Relax the clearance calculating for breaking partial register dependency.: myatsina.
I think this is a good change. The cost of inserting an xor when it is not needed is very small. The cost of failing to insert an xor when it IS needed can be huge.
DavidKreitzer added a comment to D21479: Prototype fix for replacing the SETcc/MOVZX idiom with XOR/SETcc for a 32-bit conditional set of 0,1.
DavidKreitzer retitled D21479: Prototype fix for replacing the SETcc/MOVZX idiom with XOR/SETcc for a 32-bit conditional set of 0,1 from to Prototype fix for replacing the SETcc/MOVZX idiom with XOR/SETcc for a 32-bit conditional set of 0,1.
DavidKreitzer added a comment to D20649: [X86] Add a pattern that uses GR16_ABCD rather than GR32_ABCD to avoid falsely marking whole 32 bit register as live..
DavidKreitzer added a comment to D20529: [x86, AVX] allow explicit calls to VZERO* to modify state in VZeroUpperInserter pass.
DavidKreitzer added a comment to D20518: [X86][RTM] _xabort() should not have "noreturn" attribute.
DavidKreitzer added a comment to D20468: [X86][AVX] Ensure zero-extension of _mm256_extract_epi8 and _mm256_extract_epi16.
DavidKreitzer added a comment to D20263: X86: Avoid using _chkstk when lowering WIN_ALLOCA instructions.
Thanks for the pointer to https://llvm.org/bugs/show_bug.cgi?id=26776, Reid.
I just have a couple other minor suggestions. Otherwise, LGTM.
DavidKreitzer retitled D20288: Fix for PR27750. Correctly handle the case where the FallThrough block == TBB in getFallThroughMBB. from to Fix for PR27750. Correctly handle the case where the FallThrough block == TBB in getFallThroughMBB..
DavidKreitzer added a comment to D20003: X86CallFrameOpt: a first step towards optimizing inalloca calls (PR27076).
The connection I saw between removing the _chkstk calls and this transformation is that the pushes will touch the stack in a safe order (starting at %esp, which should be safe, and progressing downwards), whereas the original stores might be in an order that starts by touching an address beyond the allocated stack.
DavidKreitzer added a comment to D17289: [X86] Fix False Data Dependency in popcnt.
Thanks for the comments, Steven! We are still working on fixing the popcnt false dependence problem, but we are planning to abandon this patch.
DavidKreitzer accepted D19261: X86 _comi_ intrinsics - Fixed lowering.
DavidKreitzer added a comment to D18046: [X86] Providing correct unwind info in function epilogue.
DavidKreitzer added a comment to D19261: X86 _comi_ intrinsics - Fixed lowering.
DavidKreitzer added inline comments to D19688: Fixes PR27241 by enabling the X86 call frame optimization for 64-bit targets.
DavidKreitzer updated the diff for D19688: Fixes PR27241 by enabling the X86 call frame optimization for 64-bit targets.
Thanks for the reviews, Reid and Hans!
DavidKreitzer retitled D19688: Fixes PR27241 by enabling the X86 call frame optimization for 64-bit targets from to Fixes PR27241 by enabling the X86 call frame optimization for 64-bit targets.
DavidKreitzer added a comment to D19592: [X86]: Quit promoting 16 bit loads to 32 bit..
DavidKreitzer accepted D19439: Optimization bisect support in X86-specific passes.
Thanks for the fixes, Andy.
DavidKreitzer added a comment to D19439: Optimization bisect support in X86-specific passes.
My comments about vzeroupper make me wonder whether we want skipFunction to be able to make the distinction between skipping a pass for OptBisect and skipping a pass for -O0. I can certainly imagine wanting to run some functionally optional "optimization" passes at -O0, e.g. a pass that can improve the size/performance of the generated code w/o negatively affecting debuggability.