[PATCH] D109531: [CSSPGO] Enable pseudo probe instrumentation in O0 mode.

2021-09-10 Thread Wei Mi via Phabricator via cfe-commits
wmi accepted this revision. wmi added a comment. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109531/new/ https://reviews.llvm.org/D109531 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

[PATCH] D109531: [CSSPGO] Enable pseudo probe instrumentation in O0 mode.

2021-09-10 Thread Wei Mi via Phabricator via cfe-commits
wmi added a comment. In D109531#2993484 , @hoy wrote: > In D109531#2993394 , @wmi wrote: > >> In D109531#2992721 , @hoy wrote: >> >>> In D109531#2992702

[PATCH] D109531: [CSSPGO] Enable pseudo probe instrumentation in O0 mode.

2021-09-09 Thread Wei Mi via Phabricator via cfe-commits
wmi added a comment. In D109531#2992721 , @hoy wrote: > In D109531#2992702 , @wenlei wrote: > >> The change makes sense given instr PGO also happens for O0. But practically, >> if a file is being built with O0, d

[PATCH] D107876: [CSSPGO] Allow the use of debug-info-for-profiling and pseudo-probe-for-profiling together

2021-08-11 Thread Wei Mi via Phabricator via cfe-commits
wmi accepted this revision. wmi added a comment. In D107876#2939691 , @hoy wrote: > In D107876#2939624 , @wmi wrote: > >> Could you remind me the discriminator difference between debug info based >> AFDO and pseu

[PATCH] D107876: [CSSPGO] Allow the use of debug-info-for-profiling and pseudo-probe-for-profiling together

2021-08-11 Thread Wei Mi via Phabricator via cfe-commits
wmi added a comment. Could you remind me the discriminator difference between debug info based AFDO and pseudo probe based AFDO? I forgot it. I am sure it is described in some patch but I havn't found it. Give me a pointer is good enough. Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D99554: [ThinLTO] During module importing, close one source module before open another one for distributed mode.

2021-03-30 Thread Wei Mi via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGd535a05ca1a6: [ThinLTO] During module importing, close one source module before open (authored by wmi). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99554/n

[PATCH] D99554: [ThinLTO] During module importing, close one source module before open another one for distributed mode.

2021-03-30 Thread Wei Mi via Phabricator via cfe-commits
wmi added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:1583 ModuleToDefinedGVSummaries[M->getModuleIdentifier()], - ModuleMap, CGOpts.CmdArgs)) { + nullptr, CGOpts.CmdArgs)) { handleAllErrors(

[PATCH] D99554: [ThinLTO] During module importing, close one source module before open another one for distributed mode.

2021-03-30 Thread Wei Mi via Phabricator via cfe-commits
wmi updated this revision to Diff 334240. wmi added a comment. Address Teresa's comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99554/new/ https://reviews.llvm.org/D99554 Files: clang/lib/CodeGen/BackendUtil.cpp clang/test/CodeGen/thinl

[PATCH] D99554: [ThinLTO] During module importing, close one source module before open another one for distributed mode.

2021-03-29 Thread Wei Mi via Phabricator via cfe-commits
wmi created this revision. wmi added a reviewer: tejohnson. Herald added subscribers: steven_wu, hiraditya, inglorion. wmi requested review of this revision. Herald added projects: clang, LLVM. Herald added a subscriber: cfe-commits. Currently during module importing, ThinLTO opens all the source

[PATCH] D95271: [CSSPGO] Passing the clang driver switch -fpseudo-probe-for-profiling to the linker.

2021-02-02 Thread Wei Mi via Phabricator via cfe-commits
wmi accepted this revision. wmi added a comment. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95271/new/ https://reviews.llvm.org/D95271 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

[PATCH] D95271: [CSSPGO] Passing the clang driver switch -fpseudo-probe-for-profiling to the linker.

2021-02-01 Thread Wei Mi via Phabricator via cfe-commits
wmi added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:609-610 + // Pass an option to enable pseudo probe emission. + if (Args.hasFlag(options::OPT_fpseudo_probe_for_profiling, + options::OPT_fno_pseudo_probe_for_profiling)) +Cmd

[PATCH] D93264: [CSSPGO] Introducing distribution factor for pseudo probe.

2021-01-15 Thread Wei Mi via Phabricator via cfe-commits
wmi accepted this revision. wmi added a comment. This revision is now accepted and ready to land. LGTM. Thanks. Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:363 const FunctionSamples *CalleeSamples; uint64_t CallsiteCount; + // Call site distribution factor to p

[PATCH] D93264: [CSSPGO] Introducing distribution factor for pseudo probe.

2021-01-14 Thread Wei Mi via Phabricator via cfe-commits
wmi added inline comments. Comment at: llvm/include/llvm/IR/PseudoProbe.h:41 // [18:3] - probe id - // [25:19] - reserved + // [25:19] - probe distribution factor // [28:26] - probe type, see PseudoProbeType hoy wrote: > wmi wrote: > > hoy wrote: > >

[PATCH] D93264: [CSSPGO] Introducing distribution factor for pseudo probe.

2021-01-14 Thread Wei Mi via Phabricator via cfe-commits
wmi added inline comments. Comment at: llvm/include/llvm/IR/PseudoProbe.h:41 // [18:3] - probe id - // [25:19] - reserved + // [25:19] - probe distribution factor // [28:26] - probe type, see PseudoProbeType hoy wrote: > wmi wrote: > > hoy wrote: > >

[PATCH] D93264: [CSSPGO] Introducing distribution factor for pseudo probe.

2021-01-13 Thread Wei Mi via Phabricator via cfe-commits
wmi added inline comments. Comment at: llvm/include/llvm/IR/PseudoProbe.h:41 // [18:3] - probe id - // [25:19] - reserved + // [25:19] - probe distribution factor // [28:26] - probe type, see PseudoProbeType hoy wrote: > hoy wrote: > > wmi wrote: > >

[PATCH] D93264: [CSSPGO] Introducing distribution factor for pseudo probe.

2021-01-12 Thread Wei Mi via Phabricator via cfe-commits
wmi added inline comments. Comment at: llvm/include/llvm/IR/PseudoProbe.h:41 // [18:3] - probe id - // [25:19] - reserved + // [25:19] - probe distribution factor // [28:26] - probe type, see PseudoProbeType The bits in discriminator is a scare resour

[PATCH] D91756: [CSSPGO] Pseudo probes for function calls.

2020-12-02 Thread Wei Mi via Phabricator via cfe-commits
wmi accepted this revision. wmi added a comment. This revision is now accepted and ready to land. In D91756#2427795 , @hoy wrote: > In D91756#2427759 , @wmi wrote: > >> Another question. Sorry for not bringing it up

[PATCH] D91756: [CSSPGO] Pseudo probes for function calls.

2020-12-01 Thread Wei Mi via Phabricator via cfe-commits
wmi added a comment. Another question. Sorry for not bringing it up earlier. When a call with probe metadata attached is inlined, the probe will be gone or it will be kept somehow? I think you want to keep the probe especially for inline instance to reconstruct the context but I didn't figure i

[PATCH] D91756: [CSSPGO] Pseudo probes for function calls.

2020-12-01 Thread Wei Mi via Phabricator via cfe-commits
wmi added inline comments. Comment at: llvm/include/llvm/IR/PseudoProbe.h:33-34 + static uint32_t packProbeData(uint32_t Index, uint32_t Type) { +assert(Index <= 0x); +assert(Type <= 0x7); +return (Index << 3) | (Type << 26) | 0x7; Add assertion

[PATCH] D91756: [CSSPGO] Pseudo probes for function calls.

2020-12-01 Thread Wei Mi via Phabricator via cfe-commits
wmi added inline comments. Comment at: llvm/lib/CodeGen/PseudoProbeInserter.cpp:50 +for (MachineBasicBlock &MBB : MF) { + MachineInstr *FirstInstr = nullptr; + for (MachineInstr &MI : MBB) { What is the usage of FirstInstr? Commen

[PATCH] D91676: Avoid redundant work when computing vtable vcall visibility

2020-11-24 Thread Wei Mi via Phabricator via cfe-commits
wmi accepted this revision. wmi added inline comments. Comment at: clang/lib/CodeGen/CGVTables.cpp:1301-1302 + // has no effect on the min visibility computed below by the recursive caller. + if (!Visited.insert(RD).second) +return llvm::GlobalObject::VCallVisibilityTransl

[PATCH] D91676: Avoid redundant work when computing vtable vcall visibility

2020-11-24 Thread Wei Mi via Phabricator via cfe-commits
wmi accepted this revision. wmi added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clang/lib/CodeGen/CGVTables.cpp:1301-1302 + // has no effect on the min visibility computed below by the recursive caller. + if (!Visited.insert(RD).second) +

[PATCH] D91676: Avoid redundant work when computing vtable vcall visibility

2020-11-24 Thread Wei Mi via Phabricator via cfe-commits
wmi added inline comments. Comment at: clang/lib/CodeGen/CGVTables.cpp:1301-1302 + // has no effect on the min visibility computed below by the recursive caller. + if (!Visited.insert(RD).second) +return llvm::GlobalObject::VCallVisibilityTranslationUnit; + ---

[PATCH] D86502: [CSSPGO] A Clang switch -fpseudo-probe-for-profiling for pseudo-probe instrumentation.

2020-11-20 Thread Wei Mi via Phabricator via cfe-commits
wmi accepted this revision. wmi added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:978-982 Opts.DebugInfoForProfiling = Args.hasFlag( OPT_fdebug_info_for_profiling, OPT_fno_debug_info_for_profi

[PATCH] D86502: [CSSPGO] A Clang switch -fpseudo-probe-for-profiling for pseudo-probe instrumentation.

2020-11-20 Thread Wei Mi via Phabricator via cfe-commits
wmi added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:978-982 Opts.DebugInfoForProfiling = Args.hasFlag( OPT_fdebug_info_for_profiling, OPT_fno_debug_info_for_profiling, false); + Opts.PseudoProbeForProfiling = + Args.hasFlag(OPT_fpseudo

[PATCH] D86193: [CSSPGO] Pseudo probe instrumentation for basic blocks.

2020-09-21 Thread Wei Mi via Phabricator via cfe-commits
wmi added a comment. Herald added a subscriber: ecnelises. The patches split from the main one look good to me. Please see if David has further comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86193/new/ https://reviews.llvm.org/D86193 ___

[PATCH] D86502: [CSSPGO] A Clang switch -fpseudo-probe-for-profiling for pseudo-probe instrumentation.

2020-08-28 Thread Wei Mi via Phabricator via cfe-commits
wmi added a comment. In D86502#2245578 , @hoy wrote: > In D86502#2245460 , @wmi wrote: > >>> The early instrumentation also allows the inliner to duplicate probes for >>> inlined instances. When a probe along with

[PATCH] D86502: [CSSPGO] A Clang switch -fpseudo-probe-for-profiling for pseudo-probe instrumentation.

2020-08-28 Thread Wei Mi via Phabricator via cfe-commits
wmi added a comment. Herald added a subscriber: danielkiss. > The early instrumentation also allows the inliner to duplicate probes for > inlined instances. When a probe along with the other instructions of a callee > function are inlined into its caller function, the GUID of the callee > funct

[PATCH] D86193: [CSSPGO] Pseudo probe instrumentation for basic blocks.

2020-08-26 Thread Wei Mi via Phabricator via cfe-commits
wmi added a comment. In D86193#2240502 , @hoy wrote: > In D86193#2240353 , @wmi wrote: > >>> There are some optimizations such as if-convert, tail call elimination, >>> that were initially blocked by the pseudo pro

[PATCH] D86193: [CSSPGO] Pseudo probe instrumentation for basic blocks.

2020-08-26 Thread Wei Mi via Phabricator via cfe-commits
wmi added a comment. > There are some optimizations such as if-convert, tail call elimination, that > were initially blocked by the pseudo probe intrinsic but is now unblocked by > fixes included in this change. With the current change we do not see perf > degradation out of SPEC and one of our

[PATCH] D86193: [CSSPGO] Pseudo probe instrumentation for basic blocks.

2020-08-23 Thread Wei Mi via Phabricator via cfe-commits
wmi added a comment. Thanks for the patch! A few questions: > probe blocks some CFG transformations that can mess up profile correlation. Can you enumerate some CFG transformations which be blocked? Is it possible that some CFG transformations being blocked are actually beneficial for later op

[PATCH] D79959: [SampleFDO] Add use-sample-profile function attribute

2020-06-02 Thread Wei Mi via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG7a6c89427c9b: [SampleFDO] Add use-sample-profile function attribute. (authored by wmi). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.ll

[PATCH] D77989: Allow disabling of vectorization using internal options

2020-04-14 Thread Wei Mi via Phabricator via cfe-commits
wmi accepted this revision. wmi added a comment. This revision is now accepted and ready to land. There is a comment about the bitwise operator, otherwise LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77989/new/ https://reviews.llvm.org/D7798

[PATCH] D77989: Allow disabling of vectorization using internal options

2020-04-13 Thread Wei Mi via Phabricator via cfe-commits
wmi added a comment. In D77989#1979725 , @wmi wrote: > The patch makes the reasoning about when the internal flags will take effect > much easier. > > One question is, with the change, the internal flags can only be used to > disable loop vectorization/s

[PATCH] D77989: Allow disabling of vectorization using internal options

2020-04-13 Thread Wei Mi via Phabricator via cfe-commits
wmi added a comment. The patch makes the reasoning about when the internal flags will take effect much easier. One question is, with the change, the internal flags can only be used to disable loop vectorization/slp vectorization/loop interleaving, but not to enable them. Can we treat them in t

[PATCH] D73307: Unique Names for Functions with Internal Linkage

2020-03-30 Thread Wei Mi via Phabricator via cfe-commits
wmi added inline comments. Comment at: clang/test/CodeGen/unique-internal-linkage-names.cpp:5 +// RUN: %clang_cc1 -triple x86_64 -x c++ -S -emit-llvm -funique-internal-linkage-names -o - < %s | FileCheck %s --check-prefix=UNIQUE + +static int glob; MaskRay wrote

[PATCH] D72538: [ThinLTO] Add additional ThinLTO pipeline testing with new PM

2020-01-10 Thread Wei Mi via Phabricator via cfe-commits
wmi accepted this revision. wmi 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/D72538/new/ https://reviews.llvm.org/D72538 ___ cfe-

[PATCH] D72538: [ThinLTO] Add additional ThinLTO pipeline testing with new PM

2020-01-10 Thread Wei Mi via Phabricator via cfe-commits
wmi added a comment. The additional pipeline testing will catch any future pass change to the pipeline. A related but separate question is do we have a way to check whether there is any other missing pass in thinlto newpm similar as that in D72386 ? =

[PATCH] D72386: [ThinLTO] pass UnrollLoops/VectorizeLoop/VectorizeSLP in CGOpts down to pass builder in ltobackend

2020-01-09 Thread Wei Mi via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG21a4710c67a9: [ThinLTO] Pass CodeGenOpts like UnrollLoops/VectorizeLoop/VectorizeSLP down to… (authored by wmi). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to com

[PATCH] D71485: [profile] Fix a crash when -fprofile-remapping-file= triggers an error

2019-12-13 Thread Wei Mi via Phabricator via cfe-commits
wmi accepted this revision. wmi 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/D71485/new/ https://reviews.llvm.org/D71485 ___ cfe-

[PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

2019-08-25 Thread Wei Mi via Phabricator via cfe-commits
wmi added a comment. In D36562#1642403 , @chill wrote: > In D36562#1641930 , @wmi wrote: > > > In D36562#1639441 , @chill wrote: > > > > > Shouldn't we disable `OPT_ffine_gra

[PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

2019-08-22 Thread Wei Mi via Phabricator via cfe-commits
wmi added a comment. In D36562#1639441 , @chill wrote: > Shouldn't we disable `OPT_ffine_grained_bitfield_accesses` only if TSAN is > active? I don't remember why it is disabled for all sanitizer modes. Seems you are right that the disabling the option

[PATCH] D42154: Don't generate inline atomics for i386/i486

2018-01-23 Thread Wei Mi via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL323281: Adjust MaxAtomicInlineWidth for i386/i486 targets. (authored by wmi, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D42154?vs=130076&i

[PATCH] D39053: [Bitfield] Add more cases to making the bitfield a separate location

2018-01-22 Thread Wei Mi via Phabricator via cfe-commits
wmi added a comment. Thanks for the size evaluation. I regarded the change as a natural and limited extension to the current fine-grain bitfield access mode, so I feel ok with the change. Hal, what do you think? https://reviews.llvm.org/D39053 ___

[PATCH] D42154: Don't generate inline atomics for i386/i486

2018-01-22 Thread Wei Mi via Phabricator via cfe-commits
wmi added a comment. In https://reviews.llvm.org/D42154#983840, @rjmccall wrote: > In https://reviews.llvm.org/D42154#977991, @wmi wrote: > > > In https://reviews.llvm.org/D42154#977975, @efriedma wrote: > > > > > The LLVM backend currently assumes every CPU is Pentium-compatible. If > > > we'r

[PATCH] D42154: Don't generate inline atomics for i386/i486

2018-01-16 Thread Wei Mi via Phabricator via cfe-commits
wmi added inline comments. Comment at: lib/Basic/Targets/X86.h:472 +CPUKind Kind = getCPUKind(Opts.CPU); +if (Kind >= CK_i586 || Kind == CK_Generic) + MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64; wmi wrote: > craig.topper wrote: > > craig.toppe

[PATCH] D42154: Don't generate inline atomics for i386/i486

2018-01-16 Thread Wei Mi via Phabricator via cfe-commits
wmi added a comment. In https://reviews.llvm.org/D42154#977975, @efriedma wrote: > The LLVM backend currently assumes every CPU is Pentium-compatible. If we're > going to change that in clang, we should probably fix the backend as well. With the patch, for i386/i486 targets, clang will genera

[PATCH] D42154: Don't generate inline atomics for i386/i486

2018-01-16 Thread Wei Mi via Phabricator via cfe-commits
wmi created this revision. wmi added reviewers: rjmccall, eli.friedman, rsmith. Herald added subscribers: cfe-commits, eraman, sanjoy. This is to fix the bug reported in https://bugs.llvm.org/show_bug.cgi?id=34347#c6 Currently, all MaxAtomicInlineWidth of x86-32 targets are set to 64. However,

[PATCH] D39053: [Bitfield] Add more cases to making the bitfield a separate location

2017-11-13 Thread Wei Mi via Phabricator via cfe-commits
wmi added a comment. I think it may be hard to fix the problem in backend. It will face the same issue of store-to-load forwarding if at some places the transformation happens but at some other places somehow it doesn't. But I am not sure whether it is acceptable to add more finegrain bitfield

[PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

2017-10-16 Thread Wei Mi via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL315915: [Bitfield] Add an option to access bitfield in a fine-grained manner. (authored by wmi). Changed prior to commit: https://reviews.llvm.org/D36562?vs=118181&id=119170#toc Repository: rL LLVM

[PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

2017-10-08 Thread Wei Mi via Phabricator via cfe-commits
wmi updated this revision to Diff 118181. wmi added a comment. Address Hal's comments. Repository: rL LLVM https://reviews.llvm.org/D36562 Files: include/clang/Basic/DiagnosticDriverKinds.td include/clang/Driver/Options.td include/clang/Frontend/CodeGenOptions.def lib/CodeGen/CGRecor

[PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

2017-10-05 Thread Wei Mi via Phabricator via cfe-commits
wmi updated this revision to Diff 117947. wmi added a comment. Address Hal's comment. Repository: rL LLVM https://reviews.llvm.org/D36562 Files: include/clang/Basic/DiagnosticDriverKinds.td include/clang/Driver/Options.td include/clang/Frontend/CodeGenOptions.def lib/CodeGen/CGRecord

[PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

2017-10-05 Thread Wei Mi via Phabricator via cfe-commits
wmi marked an inline comment as done. wmi added inline comments. Comment at: lib/CodeGen/CGRecordLayoutBuilder.cpp:444 // Add bitfields to the run as long as they qualify. -if (Field != FieldEnd && Field->getBitWidthValue(Context) != 0 && -Tail == getFieldBitOffs

[PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

2017-09-27 Thread Wei Mi via Phabricator via cfe-commits
wmi updated this revision to Diff 116896. wmi added a comment. Address Hal's comment. Separate bitfields to shards separated by the naturally-sized-and-aligned fields. Repository: rL LLVM https://reviews.llvm.org/D36562 Files: include/clang/Basic/DiagnosticDriverKinds.td include/clang/D

[PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

2017-09-26 Thread Wei Mi via Phabricator via cfe-commits
wmi added a comment. In https://reviews.llvm.org/D36562#880808, @hfinkel wrote: > You seem to be only changing the behavior for the "separatable" fields, but I > suspect you want to change the behavior for the others too. The bitfield > would be decomposed into shards, separated by the naturall

[PATCH] D38046: [Atomic][X8664] set max atomic inline/promote width according to the target

2017-09-22 Thread Wei Mi via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL313992: [Atomic][X8664] set max atomic inline width according to the target (authored by wmi). Changed prior to commit: https://reviews.llvm.org/D38046?vs=116107&id=116362#toc Repository: rL LLVM ht

[PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

2017-09-21 Thread Wei Mi via Phabricator via cfe-commits
wmi updated this revision to Diff 116232. wmi added a comment. Changes following the discussion: - Put the bitfield split logic under an option and off by default. - When sanitizer is enabled, the option for bitfield split will be ignored and a warning message will be emitted. In addition, a te

[PATCH] D38046: [Atomic][X8664] set max atomic inline/promote width according to the target

2017-09-20 Thread Wei Mi via Phabricator via cfe-commits
wmi updated this revision to Diff 116107. wmi added a comment. Herald added a subscriber: eraman. Address Eli's comments. Repository: rL LLVM https://reviews.llvm.org/D38046 Files: include/clang/Basic/TargetInfo.h lib/Basic/Targets.cpp lib/Basic/Targets/X86.h test/CodeGenCXX/atomic-i

[PATCH] D38046: [Atomic][X8664] set max atomic inline/promote width according to the target

2017-09-20 Thread Wei Mi via Phabricator via cfe-commits
wmi added inline comments. Comment at: lib/Basic/Targets/X86.h:898 + MaxAtomicPromoteWidth = 64; + MaxAtomicInlineWidth = 64; +} efriedma wrote: > wmi wrote: > > efriedma wrote: > > > I don't think we need to mess with MaxAtomicPromoteWidth? > > >

[PATCH] D38046: [Atomic][X8664] set max atomic inline/promote width according to the target

2017-09-20 Thread Wei Mi via Phabricator via cfe-commits
wmi added inline comments. Comment at: lib/Basic/Targets/X86.h:898 + MaxAtomicPromoteWidth = 64; + MaxAtomicInlineWidth = 64; +} efriedma wrote: > I don't think we need to mess with MaxAtomicPromoteWidth? > > Probably more intuitive to check "if (h

[PATCH] D38046: [Atomic][X8664] set max atomic inline/promote width according to the target

2017-09-19 Thread Wei Mi via Phabricator via cfe-commits
wmi updated this revision to Diff 115945. wmi added a comment. Address Eli's comment. Repository: rL LLVM https://reviews.llvm.org/D38046 Files: include/clang/Basic/TargetInfo.h lib/Basic/Targets.cpp lib/Basic/Targets/X86.h test/OpenMP/atomic_capture_codegen.cpp test/OpenMP/atomic_

[PATCH] D37310: [Atomic] Merge alignment information from Decl and from Type when emit atomic expression.

2017-09-08 Thread Wei Mi via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL312801: Use EmitPointerWithAlignment to get alignment information of the pointer used… (authored by wmi). Changed prior to commit: https://reviews.llvm.org/D37310?vs=114300&id=114384#toc Repository:

[PATCH] D37310: [Atomic] Merge alignment information from Decl and from Type when emit atomic expression.

2017-09-07 Thread Wei Mi via Phabricator via cfe-commits
wmi updated this revision to Diff 114300. wmi added a comment. Address John's comment. Repository: rL LLVM https://reviews.llvm.org/D37310 Files: lib/CodeGen/CGAtomic.cpp test/CodeGenCXX/atomic-align.cpp Index: test/CodeGenCXX/atomic-align.cpp ==

[PATCH] D37310: [Atomic] Merge alignment information from Decl and from Type when emit atomic expression.

2017-09-06 Thread Wei Mi via Phabricator via cfe-commits
wmi added a comment. Ping Repository: rL LLVM https://reviews.llvm.org/D37310 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37310: [Atomic] Merge alignment information from Decl and from Type when emit atomic expression.

2017-08-30 Thread Wei Mi via Phabricator via cfe-commits
wmi created this revision. Herald added a subscriber: sanjoy. This is to fix PR34347. EmitAtomicExpr will only use alignment information from Type, instead of Decl, so when the declaration of an atomic variable is marked to have the alignment equal as its size, EmitAtomicExpr don't know about i

[PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

2017-08-22 Thread Wei Mi via Phabricator via cfe-commits
wmi updated this revision to Diff 112271. wmi added a comment. Try another idea suggested by David. All the bitfields in a single run are still wrapped inside of a large integer according to CGBitFieldInfo. For the bitfields with legal integer types and aligned, change their access manner when

[PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

2017-08-10 Thread Wei Mi via Phabricator via cfe-commits
wmi added a comment. I limit the bitfield separation in the last update to only happen at the beginning of a run so no bitfield combine will be blocked. I think I need to explain more about why I change the direction and start to work on the problem in frontend. Keeping the information by gener

[PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

2017-08-10 Thread Wei Mi via Phabricator via cfe-commits
wmi updated this revision to Diff 110641. wmi added a comment. Don't separate bitfield in the middle of a run because it is possible to hinder bitfields accesses combine. Only separate bitfield at the beginning of a run. Repository: rL LLVM https://reviews.llvm.org/D36562 Files: lib/CodeG

[PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

2017-08-09 Thread Wei Mi via Phabricator via cfe-commits
wmi added a comment. In https://reviews.llvm.org/D36562#837594, @chandlerc wrote: > This has been discussed before and I still pretty strongly disagree with it. > > This cripples the ability of TSan to find race conditions between accesses to > consecutive bitfields -- and these bugs have actual