- User Since
- Jul 30 2013, 7:58 PM (207 w, 5 d)
Sat, Jul 22
I believe prior to Ivy Bridge the Intel optimization manual indicates that rep movsb/stosb was only optimized to handle 1-3 bytes. Specifically to be used to handle the remainder portion in conjunction with rep+movsd to handle the rest.
Fri, Jul 21
Please recheck all the cpp file headers. Many of them aren't 80 columns and they should probably mention the target the way the header comments do. Right now it looks like a copy and paste from Targets.cpp with only the file name changed.
I think we should drop "using namespace llvm;" from the cpp files. clang doesn't usually do that except in codegen and it doesn't look like it was required in the original Targets.cpp.
Thu, Jul 20
Just review blank lines between every function. I'm too lazy to keep marking them.
Can we go ahead and add getHighestLane to the existing class to hide the Log2_32? Maybe getNumLanes as well.
Tue, Jul 18
Mon, Jul 17
I'm considering making validateCpuIs return a std::pair with the appropriate value and a tag that indicates invalid/vendor/type/subtype. This way we can remove the target based string decoding from CodeGen by reusing the validate function(with a better name). Sema can look for the invalid tag for its error.
Sun, Jul 16
Sat, Jul 15
Accepting based on @bogner's LGTM in email reply.
I think gcc supports builtin_cpu_supports and builtin_cpu_is for non-x86. We already have an x86 only implementation of __builtin_cpu_supports so I did the same here.
Added Sanjay's compare test cases.
Ping * 4
Fri, Jul 14
Thu, Jul 13
If we were to go the route of adding a feature bit for every CPU. I wonder if we should just convert the CPU name into an enum in the target independent subtarget classes. Going through a feature bit to create an enum is just making the number of feature bits larger. Feature bits are stored in a std::bitset that last I checked was 160 bits and is stored in some of the tablegen generated data structures. Due to limitations in various place this max size of 160 is determined by the target with the most feature bits. I believe it got as high as it is because ARM or AArch64 has added a feature bit per CPU similar to this patch.
Wed, Jul 12
Sorry I accidentally lost track of this.
Tue, Jul 11
Found a mistake in my previous patch around getAvailableFeatures where I forgot to still update the Features variable that gets passed to the AMD and Intel CPU detection
I've synced what I can of this into Host.cpp now. Host.cpp now has two feature variables now because just in matching libgcc we used 31 bits. The fall back CPU detection for Intel family 6 required 3 more bits. So I gave Host.cpp a second variable. Now getAvailableFeatures in Host.cpp returns both feature variables through outparams. I've changed getAvailableFeatures here to return its single feature variable as an outparam as well to keep them somewhat consistent.
Mon, Jul 10
I can't remove the older processors from Host.cpp. Host.cpp also contains a newer processor called Goldmont that's not in libgcc yet. I think these files have slightly different goals.
Add back INTEL_CORE2 to the switch. Guess I got carried away with deleting.
Can you also add strictfp to utils/vim/syntax/llvm.vim where the other keywords are listed?
Hmm I was going off this complexity section here http://en.cppreference.com/w/cpp/container/list/size
Are any of the improved tests, cases that wouldn't be improved by my earlier proposal to demorgan logical ops on compare when one side is free to invert and the other side isn't. Which I think avoids the CSE issues.
Doesn't C++11 guarantee constant time for std::list::size?
Sun, Jul 9
I committed but forgot the differential revision line so it didn't close
I think part of another change snuck into this patch. Particularly the stuff in InstCombineSelect.cpp. So ignore that.
Thu, Jul 6
Wed, Jul 5
Mon, Jul 3
Sat, Jul 1
Reduce code duplication.
Fri, Jun 30
There's a separate review for X86.d https://reviews.llvm.org/D34828
Thu, Jun 29
Wed, Jun 28
Tue, Jun 27
Test case committed in r306416
I had a test. Maybe I forgot to git add it when I turned in. I'll check.
Mon, Jun 26
It looks like when m_SpecificInt was added in https://reviews.llvm.org/rL216586 in Aug 2014 it came with this restriction. I wonder if it was borrowed from the m_ConstantInt that writes to a uint64_t that's located right next to it in the file.
Should we OR in isGLM() every place we use isSLM() today?
Sun, Jun 25
Were you able to get the family/model/stepping info for lib/Support/Host.cpp?
Jun 23 2017
Can you add goldmont to getHostCPUName in lib/Support/Host.cpp assuming you have the family/model/stepping info for it? Also need to add a RUN line to test/CodeGen/X86/cpu.ll
Jun 22 2017
Don't add the SMAP feature flag. We should only have feature flags that are used by isel.
Jun 21 2017
Rebase on top of the one use fixes to the existing code.
Can you please repost the diff with full context using one of the commands mentioned here. http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface
Jun 20 2017
Ping * 2
We don't need a new method to copy a vector. The caller can always make a copy into a local variable first.