[PATCH] D62115: fix a issue that clang is incompatible with gcc with -H option.

skan added a comment. ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62115/new/ https://reviews.llvm.org/D62115 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62363: [X86] Enable intrinsics that convert float and bf16 data to each other

skan marked 3 inline comments as done. skan added inline comments. Comment at: lib/Headers/avx512bf16intrin.h:52 +/// +/// \headerfile +/// RKSimon wrote: > \headerfile fixed in updated patch Comment at: lib/Headers/avx512bf16intrin.h:66 +///

[PATCH] D62363: [X86] Enable intrinsics that convert float and bf16 data to each other

skan updated this revision to Diff 201601. skan added a comment. rm clang-format fix in this patch CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62363/new/ https://reviews.llvm.org/D62363 Files: include/clang/Basic/BuiltinsX86.def lib/CodeGen/CGBuiltin.cpp

[PATCH] D62363: [X86] Enable intrinsics that convert float and bf16 data to each other

skan updated this revision to Diff 201605. skan added a comment. make comments for `_mm_cvtsbh_ss` and `_mm_cvtness_sbh` better CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62363/new/ https://reviews.llvm.org/D62363 Files: include/clang/Basic/BuiltinsX86.def

[PATCH] D62363: [X86] Enable intrinsics that convert float and bf16 data to each other

skan marked an inline comment as done. skan added inline comments. Comment at: lib/Headers/avx512bf16intrin.h:37 +/// +/// This intrinsic corresponds to the EmitX86CvtBF16ToFloatExpr +/// function. craig.topper wrote: > This needs to be a comment that's useful

[PATCH] D62363: [X86] Enable intrinsics that convert float and bf16 data to each other

skan updated this revision to Diff 203170. skan added a comment. When target feature `avx512bf16` is enabled, `avx512vl` is not enabled implicity for some reasons by now. But `_mm_cvtness_sbh`, `_mm256_cvtpbh_ps` , `_mm256_maskz_cvtpbh_ps` , `_mm256_mask_cvtpbh_ps` need `avx512vl` feature. So

[PATCH] D62115: fix a issue that clang is incompatible with gcc with -H option.

skan updated this revision to Diff 203664. skan added a comment. make macro more compact CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62115/new/ https://reviews.llvm.org/D62115 Files: lib/Frontend/HeaderIncludeGen.cpp test/Frontend/clang_H_opt.c Index:

[PATCH] D62835: support -march=cooperlake (clang part)

skan created this revision. skan added reviewers: craig.topper, LuoYuanke, xiangzhangllvm, pengfei. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rC Clang https://reviews.llvm.org/D62835 Files: include/clang/Basic/X86Target.def

[PATCH] D62363: [X86] Enable intrinsics that convert float and bf16 data to each other

skan updated this revision to Diff 203330. skan added a comment. avoid implicit type conversion for intrinsic parameter CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62363/new/ https://reviews.llvm.org/D62363 Files: include/clang/Basic/BuiltinsX86.def lib/CodeGen/CGBuiltin.cpp

[PATCH] D62835: [X86] -march=cooperlake (clang)

skan updated this revision to Diff 203329. skan added a comment. uploaded a unrelated patch last time CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62835/new/ https://reviews.llvm.org/D62835 Files: include/clang/Basic/X86Target.def lib/Basic/Targets/X86.cpp

[PATCH] D62835: [X86] -march=cooperlake (clang)

skan updated this revision to Diff 203327. skan added a comment. avoid implicit type conversion for intrinsic parameter CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62835/new/ https://reviews.llvm.org/D62835 Files: include/clang/Basic/BuiltinsX86.def lib/CodeGen/CGBuiltin.cpp

[PATCH] D62363: [X86] Enable intrinsics that convert float and bf16 data to each other

skan updated this revision to Diff 203762. skan added a comment. change mask paramater' s name from `__M` to `__U` in order to be consistent with other intrinsics in the file CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62363/new/ https://reviews.llvm.org/D62363 Files:

[PATCH] D62115: fix a issue that clang is incompatible with gcc with -H option.

skan added a comment. In D62115#1547698 , @kimgr wrote: > This looks good to me, thanks! Could you help accept this patch? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62115/new/ https://reviews.llvm.org/D62115

[PATCH] D62115: fix a issue that clang is incompatible with gcc with -H option.

skan added a comment. In D62115#1538631 , @kimgr wrote: > I think the test needs a bit more work. It's essentially checking the same > thing twice to exercise the Windows path separators. > > It looks like there's already a test for `-H` in >

[PATCH] D62115: fix a issue that clang is incompatible with gcc with -H option.

skan updated this revision to Diff 205001. skan added a comment. move test case to print-header-includes.c CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62115/new/ https://reviews.llvm.org/D62115 Files: lib/Frontend/HeaderIncludeGen.cpp test/Frontend/print-header-includes.c

[PATCH] D62115: fix a issue that clang is incompatible with gcc with -H option.

skan updated this revision to Diff 200634. skan added a comment. add a test for including headfile outside current directory. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62115/new/ https://reviews.llvm.org/D62115 Files: lib/Frontend/HeaderIncludeGen.cpp

[PATCH] D62115: fix a issue that clang is incompatible with gcc with -H option.

skan marked an inline comment as done. skan added inline comments. Comment at: lib/Frontend/HeaderIncludeGen.cpp:55 + // Simplify Filename that starts with "./" + if (Filename.startswith("./")); +Filename=Filename.substr(2); lebedev.ri wrote: > skan wrote:

[PATCH] D62115: fix a issue that clang is incompatible with gcc with -H option.

skan added a comment. In D62115#1511407 , @kimgr wrote: > Also, consider `././Inputs/empty.h`. Firstly, on linux write the `clang_H_opt.c` file as #include "../Index/Inputs/empty.h" #include "Inputs/empty.h" #include "./Inputs/empty.h" #include

[PATCH] D62115: fix a issue that clang is incompatible with gcc with -H option.

skan updated this revision to Diff 200878. skan added a comment. make test case consider Windows path separators CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62115/new/ https://reviews.llvm.org/D62115 Files: lib/Frontend/HeaderIncludeGen.cpp test/Frontend/clang_H_opt.c Index:

[PATCH] D62363: Enable intrinsics that convert float and bf16 data to each other

skan created this revision. skan added reviewers: xiangzhangllvm, LuoYuanke, craig.topper. Herald added a project: clang. Herald added a subscriber: cfe-commits. Scalar version : `_mm_cvtsbh_ss` , `_mm_cvtness_sbh` Vector version: `_mm512_cvtpbh_ps` , `_mm256_cvtpbh_ps`

[PATCH] D62115: fix a issue that clang is incompatible with gcc with -H option.

skan created this revision. skan added reviewers: xiangzhangllvm, LuoYuanke, craig.topper. Herald added a project: clang. Herald added a subscriber: cfe-commits. clang behaves differently with gcc when compiling a file with -H option, gcc will omit the "./", but clang will not. I fix it to make

[PATCH] D62115: fix a issue that clang is incompatible with gcc with -H option.

skan updated this revision to Diff 200212. skan added a comment. remove ";" and format the code CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62115/new/ https://reviews.llvm.org/D62115 Files: lib/Frontend/HeaderIncludeGen.cpp test/Driver/clang_H_opt.c test/Driver/clang_H_opt.h

[PATCH] D62115: fix a issue that clang is incompatible with gcc with -H option.

skan added a comment. In D62115#1509536 , @kimgr wrote: > Should you also consider Windows path separators here? Do you mean "\\" separator? i build llvm on windows, and use clang to complie file with -H option. The output pathname also use "/" as path

[PATCH] D62115: fix a issue that clang is incompatible with gcc with -H option.

skan marked an inline comment as done. skan added inline comments. Comment at: lib/Frontend/HeaderIncludeGen.cpp:55 + // Simplify Filename that starts with "./" + if (Filename.startswith("./")); +Filename=Filename.substr(2); lebedev.ri wrote: > skan wrote:

[PATCH] D62115: fix a issue that clang is incompatible with gcc with -H option.

skan marked an inline comment as done. skan added inline comments. Comment at: lib/Frontend/HeaderIncludeGen.cpp:55 + // Simplify Filename that starts with "./" + if (Filename.startswith("./")); +Filename=Filename.substr(2); craig.topper wrote: > skan

[PATCH] D62115: fix a issue that clang is incompatible with gcc with -H option.

skan marked 2 inline comments as done. skan added inline comments. Comment at: lib/Frontend/HeaderIncludeGen.cpp:55 + // Simplify Filename that starts with "./" + if (Filename.startswith("./")); +Filename=Filename.substr(2); lebedev.ri wrote: >

[PATCH] D70157: Align branches within 32-Byte boundary

skan marked an inline comment as done. skan added inline comments. Comment at: llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp:88 +SmallVector BranchTypes; +StringRef(Val).split(BranchTypes, '-', -1, false); +for (auto BranchType : BranchTypes) {

[PATCH] D70157: Align branches within 32-Byte boundary

skan marked an inline comment as done. skan added inline comments. Comment at: llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp:88 +SmallVector BranchTypes; +StringRef(Val).split(BranchTypes, '-', -1, false); +for (auto BranchType : BranchTypes) {

[PATCH] D70157: Align branches within 32-Byte boundary

skan marked an inline comment as done. skan added inline comments. Comment at: llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp:88 +SmallVector BranchTypes; +StringRef(Val).split(BranchTypes, '-', -1, false); +for (auto BranchType : BranchTypes) {

[PATCH] D70157: Align branches within 32-Byte boundary

skan added a comment. In D70157#1745125 , @craig.topper wrote: > I've definitely passed something like > -Wl,--plugin-opt=-debug-only=isel,dagcombine on the command line before. I don't know how you could pass it succcessfully. It doesn't work for me

[PATCH] D70157: Align branches within 32-Byte boundary

skan added a comment. In D70157#1745128 , @craig.topper wrote: > In D70157#1745127 , @skan wrote: > > > In D70157#1745125 , @craig.topper > > wrote: > > > > > I've

[PATCH] D70157: Align branches within 32-Byte boundary

skan marked an inline comment as done. skan added inline comments. Comment at: llvm/lib/Target/X86/MCTargetDesc/X86BaseInfo.h:134 + /// macro-fusion. + inline FirstMFInstKind classifyFirstOpcode(unsigned Opcode) { +switch (Opcode) { craig.topper wrote: >

[PATCH] D70157: Align branches within 32-Byte boundary

skan added a comment. In D70157#1747428 , @davezarzycki wrote: > In D70157#1746793 , @MaskRay wrote: > > > On x86, the preferred function alignment is 16 > >

[PATCH] D70157: Align branches within 32-Byte boundary

skan marked 3 inline comments as done. skan added inline comments. Comment at: llvm/test/MC/X86/i386-align-branch-1a.s:1 +# RUN: llvm-mc -filetype=obj -triple i386-unknown-unknown --x86-align-branch-boundary=32 --x86-align-branch=fused-jcc-jmp --x86-align-branch-prefix-size=5

[PATCH] D70157: Align branches within 32-Byte boundary

skan added a comment. In D70157#1746793 , @MaskRay wrote: > On x86, the preferred function alignment is 16 > (https://github.com/llvm/llvm-project/blob/arcpatch-D70157/llvm/lib/Target/X86/X86ISelLowering.cpp#L1893), > which is the default function

[PATCH] D70157: Align branches within 32-Byte boundary

skan marked an inline comment as done. skan added a comment. In D70157#1755927 , @jyknight wrote: > Thanks for the comments, they help a little. But it's still somewhat > confusing, so let me write down what seems to be happening: > > - Before emitting

[PATCH] D70157: Align branches within 32-Byte boundary

skan marked an inline comment as done. skan added inline comments. Comment at: llvm/lib/Target/X86/MCTargetDesc/X86BaseInfo.h:158 +case X86::AND16i16: +case X86::AND16mr: +case X86::AND16ri: craig.topper wrote: > None of the AND/ADD/SUB instructions

[PATCH] D70157: Align branches within 32-Byte boundary

skan added a comment. In D70157#1768389 , @craig.topper wrote: > Can you please put the macro fusion changes in a separate phabricator review. > I’ll review it in the morning US time and if it all looks good we can get > that part committed while the

[PATCH] D70157: Align branches within 32-Byte boundary

skan added a comment. In D70157#1771771 , @reames wrote: > We uncovered another functional issue with this patch, or at least, the > interaction of this patch and other parts of LLVM. In our support for > STATEPOINT, PATCHPOINT, and STACKMAP we use

[PATCH] D70157: Align branches within 32-Byte boundary

skan marked an inline comment as done. skan added inline comments. Comment at: llvm/lib/MC/MCFragment.cpp:426 + case MCFragment::FT_MachineDependent: { +const MCMachineDependentFragment *MF = +cast(this); MaskRay wrote: > `const auto *`. The type is

[PATCH] D70157: Align branches within 32-Byte boundary

skan added a comment. In D70157#1786901 , @reames wrote: > Noting another issue we found in local testing (with an older version of this > patch). This interacts badly with the implicit exception mechanism in LLVM. > For that mechanism, we end up

[PATCH] D70157: Align branches within 32-Byte boundary(NOP padding)

skan updated this revision to Diff 234312. skan retitled this revision from "Align branches within 32-Byte boundary" to "Align branches within 32-Byte boundary(NOP padding)". skan edited the summary of this revision. skan added a comment. **Simplify** 1. Drop prefix padding support 2. Drop

[PATCH] D70157: Align branches within 32-Byte boundary

skan added a comment. In D70157#1777272 , @fedor.sergeev wrote: > In D70157#1776424 , @skan wrote: > > > > What if I insert explicit align(8) right *after* the sequence? > > > > If your insert explicit `.align

[PATCH] D70157: Align branches within 32-Byte boundary

skan added a comment. In D70157#1769932 , @MaskRay wrote: > I find another deficiency (infinite loop) with the current approach. > > Say, there is a `je 0` (0x0F 0x84 0x00 0x00 0x00 0x00) at byte 0x90. > (0x90+6)%32 == 0, so it ends on a 32-byte

[PATCH] D70157: Align branches within 32-Byte boundary

skan marked 3 inline comments as done. skan added inline comments. Comment at: llvm/test/MC/X86/x86-64-align-branch-1b.s:10 +# CHECK: foo: +# CHECK-NEXT:0: 64 89 04 25 01 00 00 00 movl%eax, %fs:1 +# CHECK-NEXT:8: 2e 55

[PATCH] D70157: Align branches within 32-Byte boundary

skan added a comment. In D70157#1775481 , @fedor.sergeev wrote: > In D70157#1775016 , @annita.zhang > wrote: > > > > The point is that we have explicit requirement at the start and we have a > > > lowering into

[PATCH] D70157: Align branches within 32-Byte boundary

skan added a comment. In D70157#1755927 , @jyknight wrote: > An alternative would be to simply emit NOPs before branches as needed. That > would be substantially simpler, since it would only require special handling > for a branch or a fused-branch. I

[PATCH] D70157: Align branches within 32-Byte boundary(NOP padding)

skan updated this revision to Diff 234488. skan edited the summary of this revision. skan added a comment. **Simplify** Drop the subtype of `MCBoundaryAlignFragment` and add data member `EmitNops` to indicate whether NOPs should be emitted. CHANGES SINCE LAST ACTION

[PATCH] D70157: Align branches within 32-Byte boundary(NOP padding)

skan added a comment. In D70157#1788445 , @reames wrote: > Specifically on the revised patch, I remain confused by the need for multiple > subtypes. The need for fragments *between* the potentially fused > instructions doesn't make sense to me. What I

[PATCH] D70157: Align branches within 32-Byte boundary(NOP padding)

skan updated this revision to Diff 234515. skan added a comment. 1. rename `MCBoundaryAlignFragment::hasEmitNop()` to `MCBoundaryAlignFragment::canEmitNop()` 2. reduce the number of `MCBoundaryAlignFragment` emitted as possible CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70157/new/

[PATCH] D70157: Align branches within 32-Byte boundary(NOP padding)

skan updated this revision to Diff 234520. skan added a comment. Fix a typo in `MCFragment::dump()` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70157/new/ https://reviews.llvm.org/D70157 Files: llvm/include/llvm/MC/MCAsmBackend.h llvm/include/llvm/MC/MCAssembler.h

[PATCH] D70157: Align branches within 32-Byte boundary(NOP padding)

skan marked an inline comment as done. skan added inline comments. Comment at: llvm/lib/MC/MCAssembler.cpp:1003 + // exists) also marks the end of the branch. + for (auto i = 0U, N = BF.isFused() ? 2U : 1U; + i != N && !isa(F); ++i, F = F->getNextNode()) {

[PATCH] D70157: Align branches within 32-Byte boundary(NOP padding)

skan added a comment. In D70157#1791347 , @reames wrote: > The general question is why a *range* of fragments can't be defined. > Computing the instruction size for the entire range then just requires > walking from first to last fragment in the range

[PATCH] D70157: Align branches within 32-Byte boundary(NOP padding)

skan updated this revision to Diff 234830. skan added a comment. 1. Simplify the test cases. 2. Add some comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70157/new/ https://reviews.llvm.org/D70157 Files: llvm/include/llvm/MC/MCAsmBackend.h llvm/include/llvm/MC/MCAssembler.h

[PATCH] D70157: Align branches within 32-Byte boundary(NOP padding)

skan added a comment. Do you think this patch is ready to land? @MaskRay CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70157/new/ https://reviews.llvm.org/D70157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D70157: Align branches within 32-Byte boundary(NOP padding)

skan updated this revision to Diff 234846. skan added a comment. 1. Add more tests for `!VK_NONE` cases. 2. Reduce pervasive `auto` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70157/new/ https://reviews.llvm.org/D70157 Files: llvm/include/llvm/MC/MCAsmBackend.h

[PATCH] D70157: Align branches within 32-Byte boundary(NOP padding)

skan updated this revision to Diff 234650. skan added a comment. move the code that checks if we can reuse the current `MCBoundaryAlignFragment` into the function `X86AsmBackend::getOrCreateBoundaryAlignFragment` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70157/new/

[PATCH] D70157: Align branches within 32-Byte boundary(NOP padding)

skan updated this revision to Diff 234697. skan added a comment. Add more comment CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70157/new/ https://reviews.llvm.org/D70157 Files: llvm/include/llvm/MC/MCAsmBackend.h llvm/include/llvm/MC/MCAssembler.h

[PATCH] D75017: [Driver][X86] Add helptext for malign-branch*, mbranches-within-32B-boundaries

skan created this revision. skan added reviewers: annita.zhang, LuoYuanke, craig.topper, lebedev.ri, nemanjai, MaskRay. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D75017 Files:

[PATCH] D75723: [X86] Make intrinsics _BitScan* not limited to Windows

skan updated this revision to Diff 248652. skan added a comment. Format the patch due to the warning given by pre-merge check Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75723/new/ https://reviews.llvm.org/D75723 Files:

[PATCH] D75723: [X86] Make intrinsics _BitScan* not limited to Windows

skan created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Currently if we want to use _BitScan*, we have to include intrin.h, which can only be included on Windows. Intrinsics _BitScan* are implemented by LLVM IR, so the restriction doesn't make sense.

[PATCH] D75723: [X86] Make intrinsics _BitScan* not limited to Windows

skan updated this revision to Diff 248650. skan added a comment. Fix typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75723/new/ https://reviews.llvm.org/D75723 Files: clang/include/clang/Basic/BuiltinsX86.def

[PATCH] D75723: [X86] Make intrinsics _BitScan* not limited to Windows

skan marked an inline comment as done. skan added inline comments. Comment at: clang/include/clang/Basic/BuiltinsX86.def:1904 +// BITSCAN +TARGET_BUILTIN(_BitScanForward, "UcUNi*UNi", "n", "") +TARGET_BUILTIN(_BitScanReverse, "UcUNi*UNi", "n", "") craig.topper

[PATCH] D75017: [Driver][X86] Add helptext for malign-branch*, mbranches-within-32B-boundaries

This revision was automatically updated to reflect the committed changes. Closed by commit rG6a3506a208b9: [Driver][X86] Add helptext for malign-branch*, mbranches-within-32B-boundaries (authored by skan). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D75017: [Driver][X86] Add helptext for malign-branch*, mbranches-within-32B-boundaries

skan marked an inline comment as done. skan added inline comments. Comment at: clang/include/clang/Driver/Options.td:2175 +def mbranches_within_32B_boundaries : Flag<["-"], "mbranches-within-32B-boundaries">, Flags<[DriverOption]>, Group, + HelpText<"Align branches within

[PATCH] D75017: [Driver][X86] Add helptext for malign-branch*, mbranches-within-32B-boundaries

skan updated this revision to Diff 246132. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75017/new/ https://reviews.llvm.org/D75017 Files: clang/include/clang/Driver/Options.td Index: clang/include/clang/Driver/Options.td

[PATCH] D72227: Add options for clang to align branches within 32B boundary

skan updated this revision to Diff 236945. skan marked an inline comment as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72227/new/ https://reviews.llvm.org/D72227 Files: clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Clang.cpp

[PATCH] D72227: Add options for clang to align branches within 32B boundary

skan updated this revision to Diff 237019. skan marked 5 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72227/new/ https://reviews.llvm.org/D72227 Files: clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Clang.cpp

[PATCH] D72227: [Driver][X86] Add -malign-branch* and -malign-branch-within-32B-boundaries

skan updated this revision to Diff 237491. skan retitled this revision from "Add options for clang to align branches within 32B boundary" to "[Driver][X86] Add -malign-branch* and -malign-branch-within-32B-boundaries". skan edited the summary of this revision. CHANGES SINCE LAST ACTION

[PATCH] D72463: [Driver][X86] Add -malign-branch* and -malign-branch-within-32B-boundaries

skan accepted this revision. skan added a comment. This revision is now accepted and ready to land. LGTM The principle of override here is consistent with MC (D72738 ). So I think this patch is better than D72227 . Repository:

[PATCH] D72227: [Driver][X86] Add -malign-branch* and -malign-branch-within-32B-boundaries

skan abandoned this revision. skan added a comment. The override principle of driver should keep same with MC (D72738 ). D72463 is better, so I prefer to abandon this revision. CHANGES SINCE LAST ACTION

[PATCH] D72463: [Driver][X86] Add -malign-branch* and -malign-branch-within-32B-boundaries

skan added a comment. In D72463#1813669 , @MaskRay wrote: > The commutative property (independence of options) makes option composition > easier. clangDriver makes heavy use of `getLastArg` and `hasArg`. Without the > commutative property, it would now

[PATCH] D72463: [Driver][X86] Add -malign-branch* and -malign-branch-within-32B-boundaries

skan added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2040 +if (Value.getAsInteger(10, AlignBranchBoundary) || +AlignBranchBoundary < 16 || !llvm::isPowerOf2_64(AlignBranchBoundary)) { + D.Diag(diag::err_drv_invalid_argument_to_option)

[PATCH] D72463: [Driver][X86] Add -malign-branch* and -malign-branch-within-32B-boundaries

skan added inline comments. Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:254 +def err_drv_invalid_malign_branch_EQ : Error< + "invalid argument '%0' to -malign-branch=; must be one of: %1">; + skan wrote: > The error information "must be one

[PATCH] D72463: [Driver][X86] Add -malign-branch* and -malign-branch-within-32B-boundaries

skan added a comment. In D72227 you wrote > I think options should follow these principles: > > 1. Different options are position independent. -mA -mB should be the same as > -mB -mA. > 2. -mA and -mno-A are position dependent and the last one wins. Sometimes,

[PATCH] D72227: Add options for clang to align branches within 32B boundary

skan updated this revision to Diff 237245. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72227/new/ https://reviews.llvm.org/D72227 Files: clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Clang.cpp clang/test/Driver/x86-malign-branch.c Index:

[PATCH] D72227: Add options for clang to align branches within 32B boundary

skan marked an inline comment as done. skan added inline comments. Comment at: clang/include/clang/Driver/Options.td:2200 + "Control how the assembler should align branches with NOP or segment " + "override prefix. If the boundary's size is not 0, it should be

[PATCH] D72227: Add options for clang to align branches within 32B boundary

skan marked an inline comment as done and an inline comment as not done. skan added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2045 + + if (Args.hasFlag(options::OPT_mbranches_within_32B_boundaries, +

[PATCH] D72463: [Driver][X86] Add -malign-branch* and -malign-branch-within-32B-boundaries

skan added a comment. Add revison D72225 as parent? Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2028 +if (Value.getAsInteger(10, AlignBranchBoundary) || +AlignBranchBoundary < 16 || !llvm::isPowerOf2_64(AlignBranchBoundary))

[PATCH] D72227: Add options for clang to align branches within 32B boundary

skan created this revision. skan added reviewers: craig.topper, LuoYuanke, annita.zhang, jyknight, reames, MaskRay. Herald added a project: clang. Herald added a subscriber: cfe-commits. Add an option -mbranches-within-32B-boundaries to align branches within a 32-Byte boundary to reduce the

[PATCH] D72227: Add options for clang to align branches within 32B boundary

skan updated this revision to Diff 236280. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72227/new/ https://reviews.llvm.org/D72227 Files: clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Clang.cpp clang/test/Driver/intel-align-branch.c Index:

[PATCH] D72227: Add options for clang to align branches within 32B boundary

skan updated this revision to Diff 236329. skan marked 5 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72227/new/ https://reviews.llvm.org/D72227 Files: clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Clang.cpp

[PATCH] D72227: Add options for clang to align branches within 32B boundary

skan added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2012 + ArgStringList ) { + if (const Arg *A = Args.getLastArg(options::OPT_malign_branch_boundary_EQ)) { +StringRef Value = A->getValue(); MaskRay

[PATCH] D72247: Add Triple::isX86()

skan accepted this revision. skan added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72247/new/ https://reviews.llvm.org/D72247 ___

[PATCH] D72227: Add options for clang to align branches within 32B boundary

skan marked 5 inline comments as done. skan added inline comments. Comment at: clang/include/clang/Driver/Options.td:2217 + HelpText<"Specify the maximum number of prefixes on an instruction to " + "align branches. The number should be between 0 and 4.">; +def

[PATCH] D72227: Add options for clang to align branches within 32B boundary

skan updated this revision to Diff 236547. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72227/new/ https://reviews.llvm.org/D72227 Files: clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Clang.cpp clang/test/Driver/intel-align-branch.c Index:

[PATCH] D75723: [X86] Enable intrinsics _BitScan*

skan marked an inline comment as done. skan added inline comments. Comment at: clang/lib/Headers/ia32intrin.h:421 +if (__c != 0) { \ + *(a) = (unsigned)__bsfd(__c); \

[PATCH] D75896: [X86] Support intrinsic _mm_cldemote

skan updated this revision to Diff 249548. skan added a comment. Fix typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75896/new/ https://reviews.llvm.org/D75896 Files: clang/lib/Headers/cldemoteintrin.h clang/test/CodeGen/cldemote.c

[PATCH] D75894: [X86] Support intrinsics _bextr2*

skan updated this revision to Diff 249547. skan added a comment. Address review comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75894/new/ https://reviews.llvm.org/D75894 Files: clang/lib/Headers/bmiintrin.h

[PATCH] D75723: [X86] Enable intrinsics _BitScan*

skan updated this revision to Diff 249550. skan marked an inline comment as done. skan added a comment. Make the variale name in macro start with 2 underscores Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75723/new/

[PATCH] D75723: [X86] Enable intrinsics _BitScan*

skan added inline comments. Comment at: clang/lib/Headers/ia32intrin.h:456 + __extension__({ \ +long long c = (long long)(b); \ +unsigned char d;

[PATCH] D75723: [X86] Enable intrinsics _BitScan*

skan updated this revision to Diff 249546. skan added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75723/new/ https://reviews.llvm.org/D75723 Files: clang/lib/Headers/ia32intrin.h

[PATCH] D75897: [X86] Support intrinsic _mm_broadcastsi128_si256

skan created this revision. skan added reviewers: LuoYuanke, craig.topper, RKSimon, pengfei. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D75897 Files: clang/lib/Headers/avx2intrin.h

[PATCH] D75896: [X86] Support intrinsic _mm_cldemote

skan created this revision. skan added reviewers: LuoYuanke, craig.topper, RKSimon, pengfei. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D75896 Files: clang/lib/Headers/cldemoteintrin.h

[PATCH] D75723: [X86] Enable intrinsics _BitScan*

skan updated this revision to Diff 249282. skan added a comment. Enable _BitScan* as intrinsics rather than builtin on linux Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75723/new/ https://reviews.llvm.org/D75723 Files:

[PATCH] D75894: [X86] Support intrinsics _bextr2*

skan created this revision. skan added reviewers: LuoYuanke, craig.topper, RKSimon, pengfei. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D75894 Files: clang/lib/Headers/bmiintrin.h

[PATCH] D75896: [X86] Support intrinsic _mm_cldemote

This revision was automatically updated to reflect the committed changes. Closed by commit rGab69cd0779c5: [X86] Support intrinsic _mm_cldemote (authored by skan). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75896/new/

[PATCH] D75897: [X86] Support intrinsic _mm_broadcastsi128_si256

This revision was automatically updated to reflect the committed changes. Closed by commit rG214d24e1f8e0: [X86] Support intrinsic _mm_broadcastsi128_si256 (authored by skan). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75897/new/

[PATCH] D75723: [X86] Enable intrinsics _BitScan*

skan marked an inline comment as done. skan added inline comments. Comment at: clang/lib/Headers/ia32intrin.h:421 +if (__c != 0) { \ + *(a) = (unsigned)__bsfd(__c); \

[PATCH] D75723: [X86] Make intrinsics _BitScan* not limited to Windows

skan marked an inline comment as done. skan added inline comments. Comment at: clang/include/clang/Basic/BuiltinsX86.def:1904 +// BITSCAN +TARGET_BUILTIN(_BitScanForward, "UcUNi*UNi", "n", "") +TARGET_BUILTIN(_BitScanReverse, "UcUNi*UNi", "n", "") rnk wrote: >

  1   2   >